Hi Yuppie,

Ok. I added some comments to the 'add' method of plone.z3cform:

Thanks for that!

    def add(self, object):
container = aq_inner(self.context)
        content = object
name = self.contentName
        chooser = INameChooser(container)

        # Ensure that construction is allowed

        portal_types = getToolByName(container, 'portal_types')
        fti = portal_types.getTypeInfo(content)

        if fti is not None:
            # Check add permission
            if not fti.isConstructionAllowed(container):
                raise Unauthorized(u"You are not allowed to create a %d here" % 

Inside an add form this should always be true. If it isn't true, we'll get an error anyway. So this check is redundant and can be removed.


            # Check allowable content types
            if  getattr(aq_base(container), 'allowedContentTypes', None) is not 
None and \
                    not fti.getId() in container.allowedContentTypes():
                raise Unauthorized(u"You are not allowed to create a %d here" % 

allowedContentTypes is quite expensive. I use this code instead:

   #check container constraints
   container_ti = portal_types.getTypeInfo(container)
   portal_type = content.getPortalTypeName()
   if container_ti is not None and \
           not container_ti.allowType(portal_type):
       raise ValueError('Disallowed subobject type: %s' % portal_type)


        # check preconditions
        checkObject(container, name, content)

I doubt constraints based on __setitem__ and __parent__ work in Zope 2.

Well, they do in the sense that if you have them and we have this code, we'll get an exception. They don't work generally, tough, so this may be YAGNI. It was copied from Five's Adding implementation, so I figured it should be kept if someone's relying on it.

        if IContainerNamesContainer.providedBy(container):
            # The container picks it's own names.
            # We need to ask it to pick one.
            name = chooser.chooseName(self.contentName or '', content)
            request = self.request
            name = request.get('add_input_name', name)

            if name is None:
                name = chooser.chooseName(self.contentName or '', content)
            elif name == '':
                name = chooser.chooseName('', content)
                # Invoke the name chooser even when we have a
                # name. It'll do useful things with it like converting
                # the incoming unicode to an ASCII string.
                name = chooser.chooseName(name, container)
if not name:
            raise ValueError("Cannot add content: name chooser did not provide a 

All that name handling is copied from an old version of Five's BasicAdding, which in turn is copied from old Zope 3 code. I think we should use our own code here to make sure we understand the code and it reflects our policy.

Yeah, that code's pretty nuts and doesn't make a lot of sense. I tried to refactor it when I copied it over, actually, and then found that I didn't really understand it so I left it alone.

Creating the content I set a provisional id. In the 'add' method I just use this line:

   name = chooser.chooseName(content.getId(), content)


        content.id = name
        container._setObject(name, content)
        content = container._getOb(name)
if fti is not None:

CMF trunk uses events instead of _finishConstruction.

Ah, nice. Do you think it'd be feasible to backport this, i.e. copy the event handler somewhere in Plone so long as Plone's still using an older version of CMF? Or does the new event handler rely on other changes to CMF as well?

Another option is to factor out a few things to a five.z3cform and have plone.z3cform import from it as appropriate.


plone.z3cform seems to contain Plone specific stuff. Factoring out the generic stuff to a five.z3cform package sounds good to me.

I'm CC'ing Daniel Nouri if he has an opinion. I think it should be unproblematic to test this stuff out in plone.z3cform and then have it depend on a new five.z3cform and just do convenience imports where required.

But I don't know if everybody agrees that CMF 2.2 should depend on z3c.form.

CMFCore shouldn't need to. CMFDefault may want to though.

How about we use a naming convention that's linked to the factory that's persisted in the FTI, i.e. we look for a view called "add-<factory_name>" and link to that if it's available.

The idea is that the factory name is unique and specific to the content type.

I sometimes use different factories for one content type, but a 1:1 relationship doesn't seem to be necessary for your proposal.

Different portal types that use the same factory would almost by necessity have the same add view.

This is the required 1:1 relationship. But if different portal types use the same add view, we have to pass the portal type name to the add view. (addFile currently accepts portal_type as argument to override the default portal type if necessary.)

Ah yeah. I've solved this elsewhere by having a local adapter factory that's a persistent object. It stores the portal_type and has a __call__() method that takes (context, request) to return a new add view object that's instantiated with knowledge of the portal_type.

That's for a very general add view, though. I'm not sure all types would want this. Local components are better avoided if possible.

We could make this overrideable as well, with another FTI property.

I guess I'd rather have a flexible explicit URL than an implicit URL ruled by convention.

Agree. So does this mean we want a separate property for the add view name? Should it be a simple string or a TALES thing?


Author of `Professional Plone Development`, a book for developers who
want to work with Plone. See http://martinaspeli.net/plone-book

Zope-CMF maillist  -  Zope-CMF@lists.zope.org

See http://collector.zope.org/CMF for bug reports and feature requests

Reply via email to