z3c.form render template bug

[nick]

In trying to work out how to traverse from a z3c.form Form for a simple scenario, I've uncovered something a little confusing with the Form code.

In my case the user specifies search criteria then gets a results page. I am doing the following;

from z3c.form import form
from zope.app.pagetemplate.viewpagetemplatefile import ViewPageTemplateFile

class SearchForm(form.Form):

   # usual stuff
   fields=...

   def @handleSearch(self, action):
     # process criteria and setup for displaying results
     ...
     self.template = ViewPageTemplateFile('www/results.pt')

Which breaks because of BaseForm's render() method (see comments):

def render(self):
   '''See interfaces.IForm'''
   # render content template
   if self.template is None:
     template = zope.component.getMultiAdapter((self, self.request),
         IPageTemplate)
     return template(self) # ** displays ok default edit form
   return self.template() # ** breaks results page no instance

Should the last line be self.template(self) because ViewPageTemplateFile.__call__(self, instance, *args **kw) requires a single mandatory argument?

Although ViewPageTemplateFile requires an argument, IPageTemplate doesn't mandate it so I'm wondering if there were other adapters the code's author had in mind when doing this. Seems like a bug to me anyway.

[Stephan Richter (srichter)]

On Tuesday 24 March 2009, nick wrote:
> Should the last line be self.template(self) because
> ViewPageTemplateFile.__call__(self, instance, *args **kw) requires a
> single mandatory argument?

No, because ViewPageTemplateFile can only be used on the class level. You need
to use BoundViewPageTemplateFile inside a method.

Regards,
Stephan
--
Stephan Richter
Web Software Design, Development and Training
Google me. "Zope Stephan Richter"

[nick (nick-bower)]

Thanks Stephan but I have not seen this implict assumption mentioned anywhere in either z3c.form or zope.app.pagetemplate interfaces, api, docs or tests. Sure it would probably work now reading the code but this should be explicit behvaiour enforced with a type assertion in the render method.

In fact BoundPageTemplate, now that I'm looking at it, isn't even a PageTemplate subclass, so it looks like we're using the same template attribute for either an IPageTemplate implementation or an untyped proxy object. I've got issues with that - Python may be untyped but that doesn't make this good practise anywhere.

Even the BoundPageTemplate API doesn't suggest what you've said and what it's actually used for, *args being unenforced:

    def __call__(self, *args, **kw):
        if self.im_self is None:
            im_self, args = args[0], args[1:]
        else:
            im_self = self.im_self
        return self.im_func(im_self, *args, **kw)

So I suggest 2 changes.

1) In the absense of BoundPageTemplate implementing a marker interface, I suggest changing render to at least include a type assertion to make it explicit and obvious (if we are really going to tolerate this attribute multi-typing).

    def render(self):
        '''See interfaces.IForm'''
        # render content template
        if self.template is None:
            template = zope.component.getMultiAdapter((self, self.request),
                IPageTemplate)
            return template(self)
        else:
            assert type(self.template) is type(BoundPageTemplate)
            return self.template()

2) In IForm, change the docstring to sometihng useful

    def render():
        '''Render the form, returning either acquired IPageTemplate implementation or an assigned BoundPageTemplate instance.'''

[Stephan Richter (srichter)]

On Tuesday 24 March 2009, nick wrote:
> 1) In the absense of BoundPageTemplate implementing a marker interface,
> I suggest changing render to at least include a type assertion to make
> it explicit and obvious (if we are really going to tolerate this
> attribute multi-typing).
>
>     def render(self):
>         '''See interfaces.IForm'''
>         # render content template
>         if self.template is None:
>             template = zope.component.getMultiAdapter((self, self.request),
>                 IPageTemplate)
>             return template(self)
>         else:
>             assert type(self.template) is type(BoundPageTemplate)
>             return self.template()

Why not just convert ViewPageTemplateFile to the bound version on the fly?

> 2) In IForm, change the docstring to sometihng useful
>
>     def render():
>         '''Render the form, returning either acquired IPageTemplate
> implementation or an assigned BoundPageTemplate instance.'''

Render always returns a string or iterable that produces a string. It does not
return page templates.

Regards,
Stephan
--
Stephan Richter
Web Software Design, Development and Training
Google me. "Zope Stephan Richter"

[nick (nick-bower)]

> >     def render(self):
> >         '''See interfaces.IForm'''
> >         # render content template
> >         if self.template is None:
> >             template = zope.component.getMultiAdapter((self, self.request),
> >                 IPageTemplate)
> >             return template(self)
> >         else:
> >             assert type(self.template) is type(BoundPageTemplate)
> >             return self.template()
>
> Why not just convert ViewPageTemplateFile to the bound version on the
> fly?

Well I guess because it's not obvious this is happening in the
background - I'm used to judging behaviour from a typed API I guess.
Are you suggesting no change at all - I'm not clear?

I just found this;

http://mail.zope.org/pipermail/zope3-users/2008-April/007773.html

which explains the behaviour in a way, but I'm totally uncomfortable
with the way in which I've had to "discover" this oddity the hard way.

Why can't we even have a comment here or a readme in zope.pagetemplate
explaining the attributes are expected to be differently typed depending
on if they're class or instance attributes? It's totally non-obvious to
an experienced developer picking up Zope3 for a first project.

> > 2) In IForm, change the docstring to sometihng useful
> >
> >     def render():
> >         '''Render the form, returning either acquired IPageTemplate
> > implementation or an assigned BoundPageTemplate instance.'''
>
> Render always returns a string or iterable that produces a string. It
> does not return page templates.

Iterable? Where?

[Stephan Richter (srichter)]

I think that while the current behavior might be confusing, it is not z3c.form's place to provide this documentation or enforce anything. The last suggestion was unsatisfactory, since it would break a lot of code that does use alternative templating solutions.