Opened 13 years ago
Closed 11 years ago
#16502 closed Bug (fixed)
CreateView useless error message when template_name is not specified
| Reported by: | 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)
Change History (35)
comment:1 by , 13 years ago
by , 13 years ago
| Attachment: | handle_empty_list_of_templates_r16549.diff added |
|---|
Adds check for non-emptyness for list of template_names in django.template.loader.select_template.
comment:2 by , 13 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
| Triage Stage: | Unreviewed → Accepted |
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 , 13 years ago
| Has patch: | set |
|---|
comment:4 by , 13 years ago
| Version: | 1.3 → SVN |
|---|
by , 13 years ago
| Attachment: | SingleObjectMixin-get_model.diff added |
|---|
add a get_model to SingleObjectMixin?
comment:5 by , 13 years ago
| Cc: | added |
|---|
comment:6 by , 13 years ago
| Cc: | 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 , 13 years ago
| Cc: | added |
|---|---|
| Needs tests: | set |
by , 13 years ago
| Attachment: | aggregated_patch_with_tests.diff added |
|---|
merging of patches and adding regression tests
comment:8 by , 13 years ago
| Needs tests: | unset |
|---|
I've merged my and bhustez 's patches and added regression tests for both.
comment:9 by , 13 years ago
| Cc: | added |
|---|
comment:10 by , 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.
comment:11 by , 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:14 by , 13 years ago
| Owner: | changed from to |
|---|---|
| Status: | assigned → new |
follow-up: 17 comment:16 by , 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.
comment:17 by , 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 , 12 years ago
| Owner: | changed from to |
|---|
comment:19 by , 12 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
follow-up: 21 comment:20 by , 12 years ago
| Patch needs improvement: | unset |
|---|---|
| Resolution: | → worksforme |
| Status: | assigned → closed |
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.
comment:21 by , 12 years ago
| Resolution: | worksforme |
|---|---|
| Status: | closed → new |
set form_class rather than model
https://code.djangoproject.com/attachment/ticket/16502/get_model_with_tests.3.diff#L168
follow-up: 23 comment:22 by , 11 years ago
| Patch needs improvement: | set |
|---|
follow-up: 28 comment:24 by , 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 therefore not be approved for Django.
This leaves only Bug 1 to be solved.
comment:25 by , 11 years ago
| Cc: | added |
|---|---|
| Owner: | changed from to |
| Patch needs improvement: | unset |
| Status: | new → assigned |
Here is a pull request to fix Bug 1, as described by jambonrose.
comment:26 by , 11 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
comment:28 by , 11 years ago
| Resolution: | fixed |
|---|---|
| Status: | closed → new |
determining the model based off a form specified in
form_classis 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 , 11 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
@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!
Actually, this is a crash of the debug view itself.
When Django encounters a
TemplateDoesNotExistexception, the debug view attempts to gather information about available template loaders and templates. It relies on the fact thatdjango.template.loaders.template_source_loadersis already populated (bydjango.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.