Opened 13 years ago

Closed 11 years ago

#16502 closed Bug (fixed)

CreateView useless error message when template_name is not specified

Reported by: silverghost3@… Owned by: ianawilson
Component: Generic views Version: dev
Severity: Normal Keywords: CreateView "generic view"
Cc: Silver_Ghost, anton@…, bhuztez, tinodb, preston@…, ianawilson Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

According to documentation CreateView should use %app_name%/%model_name%_form.html template by default. But if template_name is not specified it returns uninformative error:

Traceback (most recent call last):

  File "/home/kirill/workplace/projects/createview_test/lib/python2.7/site-packages/django/core/servers/basehttp.py", line 283, in run
    self.result = application(self.environ, self.start_response)

  File "/home/kirill/workplace/projects/createview_test/lib/python2.7/site-packages/django/contrib/staticfiles/handlers.py", line 68, in __call__
    return self.application(environ, start_response)

  File "/home/kirill/workplace/projects/createview_test/lib/python2.7/site-packages/django/core/handlers/wsgi.py", line 273, in __call__
    response = self.get_response(request)

  File "/home/kirill/workplace/projects/createview_test/lib/python2.7/site-packages/django/core/handlers/base.py", line 169, in get_response
    response = self.handle_uncaught_exception(request, resolver, sys.exc_info())

  File "/home/kirill/workplace/projects/createview_test/lib/python2.7/site-packages/django/core/handlers/base.py", line 203, in handle_uncaught_exception
    return debug.technical_500_response(request, *exc_info)

  File "/home/kirill/workplace/projects/createview_test/lib/python2.7/site-packages/django/views/debug.py", line 59, in technical_500_response
    html = reporter.get_traceback_html()

  File "/home/kirill/workplace/projects/createview_test/lib/python2.7/site-packages/django/views/debug.py", line 89, in get_traceback_html
    for loader in template_source_loaders:

TypeError: 'NoneType' object is not iterable

There is not anything except traceback on white background.

It will be cool if there will be default template for CreateView. If it is not a bug then note that template_name is required would be useful.

Attachments (6)

handle_empty_list_of_templates_r16549.diff (614 bytes ) - added by Silver_Ghost 13 years ago.
Adds check for non-emptyness for list of template_names in django.template.loader.select_template.
SingleObjectMixin-get_model.diff (3.4 KB ) - added by bhuztez 13 years ago.
add a get_model to SingleObjectMixin?
aggregated_patch_with_tests.diff (7.3 KB ) - added by Silver_Ghost 13 years ago.
merging of patches and adding regression tests
get_model_with_tests.diff (6.2 KB ) - added by Silver_Ghost 13 years ago.
Separate get_model-patch.
get_model_with_tests.2.diff (6.7 KB ) - added by bhuztez 13 years ago.
update patch for revision 17517
get_model_with_tests.3.diff (6.8 KB ) - added by bhuztez 13 years ago.
update patch for revision 17904

Download all attachments as: .zip

Change History (35)

comment:1 by Aymeric Augustin, 13 years ago

Actually, this is a crash of the debug view itself.

When Django encounters a TemplateDoesNotExist exception, the debug view attempts to gather information about available template loaders and templates. It relies on the fact that django.template.loaders.template_source_loaders is already populated (by django.template.loaders.find_template). But in your case, it isn't. So a new exception is raised, it overrides the initial exception, and — unfortunately — it makes it difficult to understand what really happens here.

by Silver_Ghost, 13 years ago

Adds check for non-emptyness for list of template_names in django.template.loader.select_template.

comment:2 by Silver_Ghost, 13 years ago

Owner: changed from nobody to Silver_Ghost
Status: newassigned
Triage Stage: UnreviewedAccepted

aaugustin, through information from your comment I was able to find possible place in code where additional check could be perfomed. It is django.template.loader.select_template function.

It expects list of template names and select first loadable. If there are some template names in list and no one from them couldn't be loaded then raising TemplateDoesNotExist exception is the right decision since. But if there isn't any template names in the list then >>Template<<DoesNotExist is wrong exception, I think.

So maybe checks if list is empty and raise exception with message like "I can't load any template for you because you didn't gave me any possible variants"?

comment:3 by Silver_Ghost, 13 years ago

Has patch: set

comment:4 by Silver_Ghost, 13 years ago

Version: 1.3SVN

by bhuztez, 13 years ago

add a get_model to SingleObjectMixin?

comment:5 by Anton Strogonoff, 13 years ago

Cc: anton@… added

comment:6 by bhuztez, 13 years ago

Cc: bhuztez added

function-based generic view, django.views.generic.create_update.create_object, use default template if no template_name is given. When you switch to class-based view, CreateView will not work as you has expected, since django.views.generic.edit.CreateView has no default template. This must be a bug, please give CreateView a default template in 1.3.

comment:7 by tinodb, 13 years ago

Cc: tinodb added
Needs tests: set

by Silver_Ghost, 13 years ago

merging of patches and adding regression tests

comment:8 by Silver_Ghost, 13 years ago

Needs tests: unset

I've merged my and bhustez 's patches and added regression tests for both.

comment:9 by Preston Holmes, 13 years ago

Cc: preston@… added

comment:10 by Preston Holmes, 13 years ago

Patch needs improvement: set

There are too many issues being addressed here in one patch.

The ticket is valid and seeks the addition of a default template to CreateView

Patches addressing how the template loader works, or a missing get_model method on SingleObjectMixin should have their own tickets opened and patches submitted.

by Silver_Ghost, 13 years ago

Attachment: get_model_with_tests.diff added

Separate get_model-patch.

comment:11 by Silver_Ghost, 13 years ago

Easy pickings: set
Patch needs improvement: unset

According to this discussion patch for select_template was separated to ticket:16866. Patch with get_model was left here since there are multiple ways to define which model should be used (the self.model, self.queryset or self.form_class attributes) and it justifies having a utility function for it.

Tests provided.

comment:12 by Simon Charette, 13 years ago

Just hit this issue, any chances this get attention for 1.4?

by bhuztez, 13 years ago

Attachment: get_model_with_tests.2.diff added

update patch for revision 17517

comment:13 by bhuztez, 13 years ago

update Silver_Ghost's patch for revision 17517

by bhuztez, 13 years ago

Attachment: get_model_with_tests.3.diff added

update patch for revision 17904

comment:14 by Aymeric Augustin, 13 years ago

Owner: changed from Silver_Ghost to Aymeric Augustin
Status: assignednew

comment:16 by Andrew Godwin, 12 years ago

Patch needs improvement: set

I don't like the way this patch/pull request works with ModelForms - it magically extracts a model from a ModelForm, which already needs discussion as it's new behaviour, but even worse it then passes that model out and then makes a brand new ModelForm out of it - that shouldn't happen.

in reply to:  16 comment:17 by bhuztez, 12 years ago

I don't like the way this patch/pull request works with ModelForms - it magically extracts a model from a ModelForm, which already needs discussion as it's new behaviour

Yes, but it is not a new behaviour, the function-based generic view counterpart did the same magic. I did not know whether or not the design decision had been changed to not providing a default template_name. To provide a default template_name, I don't think there is a much less magical way , given current ModelForm API.

https://github.com/django/django/blob/stable/1.4.x/django/views/generic/create_update.py#L29

but even worse it then passes that model out and then makes a brand new ModelForm out of it - that shouldn't happen.

No, it does not. If form_class already exists, it will NOT make a new ModelForm from model extracted from that, it will just return form_class you defined. The function-based generic view counterpart did the same.

comment:18 by Aymeric Augustin, 12 years ago

Owner: changed from Aymeric Augustin to nobody

comment:19 by krak3n, 11 years ago

Owner: changed from nobody to krak3n
Status: newassigned

comment:20 by krak3n, 11 years ago

Patch needs improvement: unset
Resolution: worksforme
Status: assignedclosed

I was unable to duplicate this in 1.5.1.

I created a basic model as below:

#models.py

from django.db import models

class Author(models.Model):
    name = models.CharField(max_length=100)

A basic view:

#views.py

from django.views.generic import CreateView
from .models import Author

class CreateAuthor(CreateView):
    model = Author

The traceback I got back was:

TemplateDoesNotExist at /
test_16502/author_form.html
Request Method:	GET
Request URL:	http://10.10.10.10:9000/
Django Version:	1.5.1
Exception Type:	TemplateDoesNotExist
Exception Value:	
test_16502/author_form.html
Exception Location:	/home/vagrant/django/django/django/template/loader.py in select_template, line 194
Python Executable:	/home/vagrant/.virtualenvs/django/bin/python
Python Version:	2.7.3

I think this is the correct exception that should be raised and the exception is present in the regular debug view.

Perhaps this was an issue with earlier versions of Django and it's been resolved in another ticket, though I can't hunt this down. Perhaps related to ticket:16866?

Perhaps if this is still a bug provide more information on how to reproduce it.

in reply to:  20 comment:21 by bhuztez, 11 years ago

Resolution: worksforme
Status: closednew

comment:22 by Tim Graham, 11 years ago

Patch needs improvement: set

in reply to:  22 comment:23 by magicharshit, 11 years ago

Replying to timo:

comment:24 by jambonrose, 11 years ago

This is all a little convoluted. There is no longer one problem at hand, and a little clarification might be useful.

Bug 1: get_template_names() (as defined in SingleObjectTemplateResponseMixin) is returning None, which is causing Django to throw a TemplateDoesNotExist. This should instead throw a ImproperlyConfigured error, as it does not have the information to determine the template file to load. This is more eloquently described in #18853 (marked as duplicate to this topic)

Bug 2: The TemplateDoesNotExist exception is causing the server error message, as detailed (and solved) in #21058.

Feature Request 1: Creating a CBGV by only overriding the form_class variable. The patch provided creates the ability to do so, but does not actually solve the bugs detailed.

I spoke to Russel about the possibility of the new feature. Unfortunately, determining the model based off a form specified in form_class is not desirable, because this assumes the form is a ModelForm, which may not be the case. As such, this feature (and patch) will not be approved for Django.

This leaves only Bug 1 to be solved.

Last edited 11 years ago by jambonrose (previous) (diff)

comment:25 by ianawilson, 11 years ago

Cc: ianawilson added
Owner: changed from krak3n to ianawilson
Patch needs improvement: unset
Status: newassigned

Here is a pull request to fix Bug 1, as described by jambonrose.

https://github.com/django/django/pull/1580

Last edited 11 years ago by ianawilson (previous) (diff)

comment:26 by Russell Keith-Magee <russell@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 9b2dc12b8332389d1bfb9e83123a088a084a6a47:

Merge pull request #1580 from ianawilson/ticket_16502

Fixed #16502 -- Fixed a TemplateDoesNotExist error that should be an ImproperlyConfigured.

Assistance on the patch from #jambronrose.

comment:27 by Russell Keith-Magee <russell@…>, 11 years ago

In 99952bab3008622b05613ed6ec54c3e1c63c0a1d:

[1.6.x] Merge pull request #1580 from ianawilson/ticket_16502

Fixed #16502 -- Fixed a TemplateDoesNotExist error that should be an ImproperlyConfigured.

Assistance on the patch from #jambronrose.

Backport of 9b2dc12b8332389d1bfb9e83123a088a084a6a47 from master.

in reply to:  24 comment:28 by bhuztez, 11 years ago

Resolution: fixed
Status: closednew

determining the model based off a form specified in form_class is not desirable, because this assumes the form is a ModelForm, which may not be the case.

CreateView inherits ModelFormMixin. This already assumes form is a ModelForm.

comment:29 by Tim Graham, 11 years ago

Resolution: fixed
Status: newclosed

@bhuztez I'm going to re-close this and ask that you please open a new ticket since we try to have one issue per ticket. I think the feature request and patch look reasonable, but it's a bit difficult to follow the conversation. Could you please a new ticket that summarizes the details and include the most recent patch? I think the existing patch also needs documentation. Thanks!

Note: See TracTickets for help on using tickets.
Back to Top