Re: [Zope-CMF] [dev] 'add' actions and views - a proposal
yuppie wrote at 2008-9-21 13:48 +0200: ... Proposed CMFCore TypesTool changes -- 7.) listActions of the types tool returns add actions for *all* portal types. +1 - this change is already done, right? Yes. Checked in yesterday. But hopefully, the different add actions can be grouped easily. We have portals with many dozens of add actions and do not want to present them on a single page (but have them grouped on several pages according to foci -- we are currently use appropriate categorys for the grouping). -- Dieter ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] [dev] 'add' actions and views - a proposal
Hi! Martin Aspeli wrote: yuppie wrote: Martin Aspeli wrote: Wichert Akkerman wrote: Why not a ++add++ traverser? Aren't traversed supposed to be used for that kind of thing? Or does a view gives us something here that a traverser doesn't? Namespace traversal adapters are similar to IPublishTraverse solutions. The difference is that the namespace traversal adapter normally returns something containerish from which traversal continues. I think it's intended mostly as a redirect to a different traversal namespace, e.g. in the way that plone.app.portlets has namespaces for portlet managers. I don't think a containerish return value is characteristic for namespace adapters. For example the ++view++ traverser usually doesn't return something containerish. I now implemented an ++add++ traverser in my sandbox and it seems to work fine. Cool. :) Let us know when it's checked in, I'd love to have a look at it! Ok. I checked in all my local changes. AFAICS everything works fine, but tests are still missing. Please note that so far only File has a full add view. All other content types use the fallback add view. I still use the pattern that adapts ITypeInformation as well. Our add views are anyway Zope 2 specific, so I don't think requiring explicit Zope 2 security declarations is unacceptable. All other solutions have also their drawbacks. Cheers, Yuppie ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] [dev] 'add' actions and views - a proposal
Hi Martin! Martin Aspeli wrote: yuppie wrote: Proposed CMFDefault changes --- 1.) CMF add views adapt not only container and request, but also the type info object. This way the views can't be accessed directly and have self.fti available. This is quite interesting, and possibly necessary. However, it means that CMF add views are not just views and will need to be registered differently to other views (i.e. you can't just use browser:page / which also means that you won't get the Five security treatment etc). Yes. This causes more problems than it solves. I think I found a much better solution: CMF add views are registered for a special layer called IAddViewLayer. Like any other layer, IAddViewLayer extends IBrowserRequest. And it defines an additional 'ti' attribute for the request. Like before views can't be accessed directly and have self.ti available. (I now use 'ti' instead of 'fti' because we have other type info implementations than FactoryTypeInformation.) 3.) A traversal hook looks up the type info based on the view name. And then returns the corresponding add form: queryMultiAdapter((container, request, fti), name=fti.factory) Why do we need to *both* adapt the FTI and use the factory name as the view name? Because we want to use different views for different content factories *and* pass the type info to the view. This way we don't depend on an environment that sets the ti value later. And all view methods work from the beginning without fallback code for a missing ti. As mentioned above, I now propose to pass in the type info as part of the request object. 6.) An IPublishTraverse adapter is used for IFolderish objects. It tries to resolve names starting with '+' and falls back to DefaultPublishTraverse behavior if no add view is found. This way URLs like http://www.example.org/folder/+PortalType are supported. -1 Or maybe -10 now that I think about it a bit more... I also started with -1, but thinking a bit more about it my vote is +1. This is pretty invasive. It'd make it impossible to have another IPublishTraverse adapter for any IFolderish without either subclassing from the CMFDefault one or breaking all add forms. Shrug. That is how the IPublishTraverse adapter works. There are many use cases for the IPublishTraverse hook and only one available slot. You never can be sure that this hook is free. If you write your own IPublishTraverse adapter, you always have to look at the code base and subclass those adapters you want to use. Rather than invent a pseudo namespace with a naming convention, why not use a proper namespace, i.e. http://www.example.org/folder/@@add/PortalType ? What's the difference between a 'pseudo' namespace and a proper namespace? The 'add' view you propose is a 'pseudo' view. It just exists to be bypassed, not to be returned. This is short, simple and allows alternative implementations if people want to use a different name (e.g. the @@add-deterity-content traverser in Dexterity), and does not require a clunky traverser for all IFolderish. Instead, you register the @@add view for IFolderish and have it implement IPublishTraverse as well. We have a trade-off here: clunky traverser vs. clunky URL. I don't want to have names like '@@add-dexterity-content', '@@add-cmf-content' or '@@resize-plone-image' in my URLs. '@@add' looks a bit more generic, but it isn't. '+PortalType' works with any add view implementation. If you hardcode the portal type, you can register the add view directly for the default skin. If you want to use the CMF traversal or dexterity traversal, you can still support the same URL. Following your proposal, we have '@@addPortalType' for simple add views, '@@add/PortalType' for CMF traversal and '@@add-dexterity-content/PortalType' for dexterity traversal. I don't think that's what IPublishTraverse is made for. Are the IBaseObject traversers used by Archetypes and plone.app.imaging also 'bad'? Or what's the big difference to the adapter I propose? Proposed CMFCore TypesTool changes -- 7.) listActions of the types tool returns add actions for *all* portal types. +1 - this change is already done, right? Yes. Checked in yesterday. 8.) If no add view expression is defined in the type info, a default add view URL is returned. -0 Would it not be better to push this logic higher up, i.e. in the view code that's actually using the add views? Can't follow you here. How should that work? The default for CMFCore is probably not going to be a suitable fallback for Plone, for example, which means we either need to customise/override or ensure that all types always have such an add action. With a default value CMFDefault needs no migration code and copying type infos is easier. But I agree that it might be useful to make it customizable. Maybe we should use a types tool property
Re: [Zope-CMF] [dev] 'add' actions and views - a proposal
Hi Yuppie, 1.) CMF add views adapt not only container and request, but also the type info object. This way the views can't be accessed directly and have self.fti available. This is quite interesting, and possibly necessary. However, it means that CMF add views are not just views and will need to be registered differently to other views (i.e. you can't just use browser:page / which also means that you won't get the Five security treatment etc). Yes. This causes more problems than it solves. I think I found a much better solution: CMF add views are registered for a special layer called IAddViewLayer. Like any other layer, IAddViewLayer extends IBrowserRequest. And it defines an additional 'ti' attribute for the request. Like before views can't be accessed directly and have self.ti available. (I now use 'ti' instead of 'fti' because we have other type info implementations than FactoryTypeInformation.) I'm not sure I like this much more. It involves adding a marker interface to the request conditionally during traversal. You'll possibly run into funny sequence dependent conditions if you want to customise the add view for a particular theme browser layer. My preference would be: - Define an interface IFTIAwareView that has an 'fti' property - Define a traversal view (@@add) that does this kind of thing on traversal: addview = queryMultiAdapter((self.context, request), name=name) if IFTIAwareView.providedBy(addview): addview.fti = fti return addview.__of__(self.context) or something to that effect. This keeps the view that the developer writes close to a real view that can be customised in the normal ways. Of course, it can only be traversed to via the traversal adapter, but I think this'll be the case no matter what. 3.) A traversal hook looks up the type info based on the view name. And then returns the corresponding add form: queryMultiAdapter((container, request, fti), name=fti.factory) Why do we need to *both* adapt the FTI and use the factory name as the view name? Because we want to use different views for different content factories *and* pass the type info to the view. This way we don't depend on an environment that sets the ti value later. And all view methods work from the beginning without fallback code for a missing ti. The addview definitely needs to know which FTI it's being used for. However, in the case above, you could just have it be an unnamed adapter, since self.fti.factory will always equal self.__name__ in the view, no? As mentioned above, I now propose to pass in the type info as part of the request object. Right. I'm not sure this is naturally something that belongs in the request, though. 6.) An IPublishTraverse adapter is used for IFolderish objects. It tries to resolve names starting with '+' and falls back to DefaultPublishTraverse behavior if no add view is found. This way URLs like http://www.example.org/folder/+PortalType are supported. -1 Or maybe -10 now that I think about it a bit more... I also started with -1, but thinking a bit more about it my vote is +1. Why? Why is the +PortalType convention so important? It's not used elsewhere, and it means dictating a convention that developers must know about and follow. Moreover, this convention currently means something else in Plone. :) In plone.app.contentrules, for example, we have a +condition view that's very similar to the + view; it provides IConditionAdding that's analogous to IAdding (provided by +). I'm pretty sure your proposed traverser would conflict and screw up the +condition (and +action and +portlet) views we have there already, necessitating complex workarounds that would need to be aware of the semantics of multiple packages. This is pretty invasive. It'd make it impossible to have another IPublishTraverse adapter for any IFolderish without either subclassing from the CMFDefault one or breaking all add forms. Shrug. That is how the IPublishTraverse adapter works. There are many use cases for the IPublishTraverse hook and only one available slot. So you want CMF to steal that slot for *all* folders? You never can be sure that this hook is free. If you write your own IPublishTraverse adapter, you always have to look at the code base and subclass those adapters you want to use. Well, you can avoid writing the one adapter for a very generic interface. In the @@add case, CMF provides *one* view (i.e. we reserve the name @@add for any IFolderish) and traversal continues from that view. Put differently, traversal from a folder is complex, but also well established. You travese into attributes/contained objects or views. Traversal from a new @@add view does not have any pre-existing semantics. We can choose exactly what we do with the traversal sub-path. We don't need to derive from the default IPublishTraverse at all. Rather than invent a pseudo namespace with a naming
Re: [Zope-CMF] [dev] 'add' actions and views - a proposal
Previously Martin Aspeli wrote: Wichert Akkerman wrote: Previously Martin Aspeli wrote: Hi Yuppie, 1.) CMF add views adapt not only container and request, but also the type info object. This way the views can't be accessed directly and have self.fti available. This is quite interesting, and possibly necessary. However, it means that CMF add views are not just views and will need to be registered differently to other views (i.e. you can't just use browser:page / which also means that you won't get the Five security treatment etc). Yes. This causes more problems than it solves. I think I found a much better solution: CMF add views are registered for a special layer called IAddViewLayer. Like any other layer, IAddViewLayer extends IBrowserRequest. And it defines an additional 'ti' attribute for the request. Like before views can't be accessed directly and have self.ti available. (I now use 'ti' instead of 'fti' because we have other type info implementations than FactoryTypeInformation.) I'm not sure I like this much more. It involves adding a marker interface to the request conditionally during traversal. You'll possibly run into funny sequence dependent conditions if you want to customise the add view for a particular theme browser layer. My preference would be: - Define an interface IFTIAwareView that has an 'fti' property - Define a traversal view (@@add) that does this kind of thing on traversal: Why not a ++add++ traverser? Aren't traversed supposed to be used for that kind of thing? Or does a view gives us something here that a traverser doesn't? Namespace traversal adapters are similar to IPublishTraverse solutions. The difference is that the namespace traversal adapter normally returns something containerish from which traversal continues. I think it's intended mostly as a redirect to a different traversal namespace, e.g. in the way that plone.app.portlets has namespaces for portlet managers. The containerish thing is just a lookup-mechanism, which could be a very simple thing to figure out the right add view, which shouldn't be more than half a dozen lines of code. It feels like a perfect fit to me. Wichert. -- Wichert Akkerman [EMAIL PROTECTED]It is simple to make things. http://www.wiggy.net/ It is hard to make things simple. ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] [dev] 'add' actions and views - a proposal
Previously Martin Aspeli wrote: Wichert Akkerman wrote: Previously Martin Aspeli wrote: Wichert Akkerman wrote: Previously Martin Aspeli wrote: Hi Yuppie, 1.) CMF add views adapt not only container and request, but also the type info object. This way the views can't be accessed directly and have self.fti available. This is quite interesting, and possibly necessary. However, it means that CMF add views are not just views and will need to be registered differently to other views (i.e. you can't just use browser:page / which also means that you won't get the Five security treatment etc). Yes. This causes more problems than it solves. I think I found a much better solution: CMF add views are registered for a special layer called IAddViewLayer. Like any other layer, IAddViewLayer extends IBrowserRequest. And it defines an additional 'ti' attribute for the request. Like before views can't be accessed directly and have self.ti available. (I now use 'ti' instead of 'fti' because we have other type info implementations than FactoryTypeInformation.) I'm not sure I like this much more. It involves adding a marker interface to the request conditionally during traversal. You'll possibly run into funny sequence dependent conditions if you want to customise the add view for a particular theme browser layer. My preference would be: - Define an interface IFTIAwareView that has an 'fti' property - Define a traversal view (@@add) that does this kind of thing on traversal: Why not a ++add++ traverser? Aren't traversed supposed to be used for that kind of thing? Or does a view gives us something here that a traverser doesn't? Namespace traversal adapters are similar to IPublishTraverse solutions. The difference is that the namespace traversal adapter normally returns something containerish from which traversal continues. I think it's intended mostly as a redirect to a different traversal namespace, e.g. in the way that plone.app.portlets has namespaces for portlet managers. The containerish thing is just a lookup-mechanism, which could be a very simple thing to figure out the right add view, which shouldn't be more than half a dozen lines of code. It feels like a perfect fit to me. I don't feel particularly strongly either way, so long as there's an actual namespace rather than a naming convention and we avoid an IPublishTraverse adapter for all IFolderish. ++add++PortalType is a bit uglier than /@@add/PortalType IMHO, but it's a transient URL so it doesn't really matter. It makes it more explicit that there is no real @@add 'thing' that is traversed over. I think it's worth finding out why we have +/IAdding being a view and not a namespace traversal adapter, though. It feels that things like ++skin++ or ++vh++ are a bit different to ++add++, though perhaps not. The + naming for IAdding has always been a mystery to me. It feels very out of place considering that it is just about traversing into a add-view namespace. Wichert. -- Wichert Akkerman [EMAIL PROTECTED]It is simple to make things. http://www.wiggy.net/ It is hard to make things simple. ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] [dev] 'add' actions and views - a proposal
Wichert Akkerman wrote: Previously Martin Aspeli wrote: I don't feel particularly strongly either way, so long as there's an actual namespace rather than a naming convention and we avoid an IPublishTraverse adapter for all IFolderish. ++add++PortalType is a bit uglier than /@@add/PortalType IMHO, but it's a transient URL so it doesn't really matter. It makes it more explicit that there is no real @@add 'thing' that is traversed over. I think it's worth finding out why we have +/IAdding being a view and not a namespace traversal adapter, though. It feels that things like ++skin++ or ++vh++ are a bit different to ++add++, though perhaps not. The + naming for IAdding has always been a mystery to me. It feels very out of place considering that it is just about traversing into a add-view namespace. In Zope 3 '+' *is* a real page with content. Maybe that's the reason? Anyway. I like the idea to use a traverser. It's more explicit and if you want different URLs you can hide them behind aliases. Cheers, Yuppie ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] [dev] 'add' actions and views - a proposal
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Sep 19, 2008, at 14:12 , yuppie wrote: If there are no objections, I'll make the proposed changes on the trunk. Thanks for taking the time to discuss and think through all of this. @ Jens: When exactly do you want to make the beta release? When you're done? ;-) Remember, I only suggested making the release a few weeks ago because I had some sprint-time I could devote to cleanup and bug squashing as a preparation. No one specifically asked for it (yet). jens -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.8 (Darwin) iEYEARECAAYFAkjTq/kACgkQRAx5nvEhZLJO2gCfTZbwuKTQBUAbq1VNIHdsi50J sMMAn0A2FDSN3GN0P/oDRV8xJWxlXljS =06Ys -END PGP SIGNATURE- ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
