Using class based generic view DetailView with a ModelForm reveals a bug - how to proceed?
Asked Answered
S

1

7

I've been impressed how rapidly a functional website can go together with generic views in the tutorials. Also, the workflow for form processing is nice. I used the ModelForm helper class to create a form from a model I made and was delighted to see that so much functionality came together. When I used the generic list_detail.object_detail I was disappointed that all that I could display were fields individually. I knew the ModelForm class contained information for rendering, so I wanted to use the ModelForm with a generic view.

I was asking around on stackoverflow to get some direction, and appreciate the answers and comments from several posters. I've figured out how to get this to work, but there is a bug in DetailView. The solution includes a workaround.

To use a ModelView with the generic view and get all the fields to render automatically the following works:

Create a project, and in it create application inpatients.

If you have

# inpatients/models.py

class Inpatient(models.Model):
    last_name = models.CharField(max_length=30)
    first_name = models.CharField(max_length=30,blank=True)
    address = models.CharField(max_length=50,blank=True)
    city = models.CharField(max_length=60,blank=True)
    state = models.CharField(max_length=30,blank=True)
    DOB = models.DateField(blank=True,null=True)
    notes = models.TextField(blank=True)

    def __unicode__(self):
        return u'%s, %s %s' % (self.last_name, self.first_name, self.DOB)

class InpatientForm(ModelForm):
    class Meta:
        model = Inpatient

and

# inpatients/views.py

from django.http import HttpResponse, HttpResponseRedirect
from django.shortcuts import render_to_response
from django.views.generic import DetailView
from portal.inpatients.models import *

def formtest(request):
    if request.method == 'POST':
        form = InpatientForm(request.POST)
        if form.is_valid():
            form.save()
            return HttpResponseRedirect('/inpatients')
    else:
        form = InpatientForm()
    return render_to_response("formtest.html", {'form': form})

class FormDetailView(DetailView):
    model=Inpatient
    context_object_name='inpatient'   # defines the name in the template
    template_name_field='inpatient_list_page.html'

    def get_object(self):
        inpatient=super(FormDetailView,self).get_object()
        form=InpatientForm(instance=inpatient)
        return form

    def get_template_names(self):
        return ['inpatient_list_page.html',]

and

#urls.py

from django.conf.urls.defaults import patterns, include, url
from django.views.generic import ListView
from portal.inpatients.models import Inpatient, InpatientForm
from portal.inpatients.views import FormDetailView

urlpatterns = patterns('',
    (r'^formtest/$','portal.inpatients.views.formtest'),
    (r'^inpatients/$', ListView.as_view(
        model=Inpatient, template_name='inpatient_list_page.html')),
    (r'^inpatient-detail/(?P<pk>\d+)/$', FormDetailView.as_view()),
)

# with a template containing

{% block content %}
    <h2>Inpatients</h2>
    <ul>
        {% for aninpatient in object_list %}
            <li><a href='/inpatient-detail/{{ aninpatient.id }}/'>
            {{ aninpatient }}, id={{ aninpatient.id }}</a></li>
        {% endfor %}
    </ul>
    {{ inpatient.as_p }}
{% endblock %}
# Yeah, kind of hokey. The template is for both the list view and detail view. 
# Note how the form is rendered with one line - {{ inpatient.as_p }}

it works. The instructions for using class based generic views lives at https://docs.djangoproject.com/en/1.3/topics/class-based-views/ Instructions there are pretty clear. The key to making things work is to redefine get_object. In the documentation under the section "Performing extra work" it nicely describes how to do this, the steps being to call the original version of get_object, and then to the extra work. The bit that I realized is that the return object can be a ModelForm object. The object that get_object returns goes straight into the template in a render. By taking the retrieved inpatient object and running it through InpatientForm it can be passed to a view as a form which then renders itself.

As to the bug: The bug in DetailView is that the get_template_names function tries to make a template name from a structure that does not exist. In https://code.djangoproject.com/browser/django/trunk/django/views/generic/detail.py on lines 127 to 140 we have within SingleObjectTemplateResponseMixin.get_template_names:

127        # The least-specific option is the default <app>/<model>_detail.html;
128         # only use this if the object in question is a model.
129         if hasattr(self.object, '_meta'):
130             names.append("%s/%s%s.html" % (
131                 self.object._meta.app_label,
132                 self.object._meta.object_name.lower(),
133                 self.template_name_suffix
134             ))
135         elif hasattr(self, 'model') and hasattr(self.model, '_meta'):
136             names.append("%s/%s%s.html" % (
137                 self.model._meta.app_label,
138                 self.model._meta.object_name.lower(),
139                 self.template_name_suffix
140             ))

The error is that the code on line 131 is executed and dies with error message <'ModelFormOptions' object has no attribute 'app_label'>. I conclude that the _meta object is defined. I suppose that the problem is that in a ModelForm the class Meta is defined. That Meta probably doesn't have the fields set that are expected. The workaround is just to rewrite get_template_names and return the correct template.

I'm new to Django and Python. I appreciate the answers and comments by the contributors at the following previous questions I asked. ( Putting links in list_detail.object_list to list_detail.object_detail, Using form in object_detail, Rolling your own generic views in Django)

What should I do to report the bug?

Stegall answered 3/7, 2011 at 16:27 Comment(2)
I don't think this is a bug, and I do think get_object should always return model instance not ModelForm instance. Try using editing CBV.Talc
I think it is a bug for several reasons. The documentation does not say it is invalid. The test for valid data before the assignment tests for the existence of _meta rather than the actual fields. The routine that is looking for the template didn't find the template. Additionally, on the principal of Don't Repeat Yourself, the ModelForm should be able to be delivered to a template for rendering.Stegall
L
2

You are right I believe. This is a bug which stems from the fact that both ModelForm and Models have a _meta attribute. This same bug would exhibit itself anytime an object is returned from get_object() that contains a _meta attribute.

get_object does not have to return a Model instance. You can confirm this by looking at the source for DetailView and reading it's docstring:

class DetailView(SingleObjectTemplateResponseMixin, BaseDetailView):
    """
    Render a "detail" view of an object.

    By default this is a model instance looked up from `self.queryset`, but the
    view will support display of *any* object by overriding `self.get_object()`.
    """

Notice that the doc string explicitly says that any object is supported by overriding self.get_object().

Another piece of corroborating evidence is from the location where this bug itself occurs which is the get_template_names method of SingleObjectTemplateResponseMixin.

    # The least-specific option is the default <app>/<model>_detail.html;
    # only use this if the object in question is a model.
    if hasattr(self.object, '_meta'):
        names.append("%s/%s%s.html" % (
            self.object._meta.app_label,
            self.object._meta.object_name.lower(),
            self.template_name_suffix
        ))
    elif hasattr(self, 'model') and hasattr(self.model, '_meta'):
        names.append("%s/%s%s.html" % (
            self.model._meta.app_label,
            self.model._meta.object_name.lower(),
            self.template_name_suffix
        ))

Again looking at this code, the comment itself say "If the object in question is a model". From this comment we can infer that the object doesn't always have to be a model.

However if you are trying to create a view that allows someone to edit/create/delete a model you really should have a look at the Editing Views which include FormView, CreateView, EditView and DeleteView. You can see more information for these at https://docs.djangoproject.com/en/1.3/ref/class-based-views/#editing-views.

To answer the question as to how to report the bug, you should follow the guidelines detailed at https://docs.djangoproject.com/en/1.3/internals/contributing/#reporting-bugs.

Louiselouisette answered 4/7, 2011 at 17:12 Comment(3)
I agree with the edit/delete issue. I envision an automatic display as I have followed by controls on that page to bring up the edit/delete functions. Of course, I'll be using the generic views as much as possible. I appreciate your corroborating my sense of what should be allowed. If Django goes the way of many projects it will add additional complexity and then when a critical point is reached a rewrite that is simpler and allows greater functionality will emerge. Already we've gone from function based to class based. I'm sure lots of Django has already gone through that. Thanks, SteveStegall
Still not convinced this is a bug. You should not use DetailView where FormView is more appropriate for one thing. As for get_template_names they could have tested with isinstance in which case you should return Model instance in order to figure out template name. This way you can return anything with _meta and be able to get the template name. Except for form. You should use form views with it.Talc
This is a bug because it will occur with ANY class that has a _meta instance that isn't a model. It doesn't have to be a form class (which are more appropriate to use a FormView for.Louiselouisette

© 2022 - 2024 — McMap. All rights reserved.