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 , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
follow-up: 21 comment:20 by , 11 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 , 11 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 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_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 , 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
TemplateDoesNotExist
exception, the debug view attempts to gather information about available template loaders and templates. It relies on the fact thatdjango.template.loaders.template_source_loaders
is 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.