Opened 19 years ago

Closed 16 years ago

#1972 closed defect (fixed)

Models with no fields create errors in Admin interface

Reported by: ttate@… Owned by: nobody
Component: contrib.admin Version: dev
Severity: critical Keywords: models onetoone foreignkey
Cc: freakboy3742@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

With the following model:

from django.db import models

class List(models.Model):
	# Has many Item objects
	pass

class ListThing(models.Model):
	list = models.OneToOneField(List)
	
	class Admin:
		pass

class Item(models.Model):
	x = models.ForeignKey(X)

The admin interface crashes when you try to add a new List to a ListThing with the following:

Traceback (most recent call last):
File "/Library/Frameworks/Python.framework/Versions/2.4/lib/python2.4/site-packages/Django-0.91-py2.4.egg/django/core/handlers/base.py" in get_response
  74. response = callback(request, *callback_args, **callback_kwargs)
File "/Library/Frameworks/Python.framework/Versions/2.4/lib/python2.4/site-packages/Django-0.91-py2.4.egg/django/contrib/admin/views/decorators.py" in _checklogin
  54. return view_func(request, *args, **kwargs)
File "/Library/Frameworks/Python.framework/Versions/2.4/lib/python2.4/site-packages/Django-0.91-py2.4.egg/django/views/decorators/cache.py" in _wrapped_view_func
  40. response = view_func(request, *args, **kwargs)
File "/Library/Frameworks/Python.framework/Versions/2.4/lib/python2.4/site-packages/Django-0.91-py2.4.egg/django/contrib/admin/views/main.py" in add_stage
  299. return render_change_form(model, manipulator, c, add=True)
File "/Library/Frameworks/Python.framework/Versions/2.4/lib/python2.4/site-packages/Django-0.91-py2.4.egg/django/contrib/admin/views/main.py" in render_change_form
  196. field_sets = opts.admin.get_field_sets(opts)

  AttributeError at /admin/picalendar/list/add/
  'NoneType' object has no attribute 'get_field_sets'

Always reproducible. Using latest SVN (2967). This might be a dupe of ticket 1939, but I'm submitting as a separate ticket just to be sure.

Attachments (2)

management.diff (818 bytes ) - added by James Bennett 18 years ago.
Patch which makes syncdb raise ImproperlyConfigured on a model with no fields
management.2.diff (722 bytes ) - added by James Bennett 18 years ago.
Better patch; this puts it in model validation

Download all attachments as: .zip

Change History (20)

comment:1 by ttate@…, 19 years ago

Whoops - typo in simple test case. Corrected:

from django.db import models

class List(models.Model):
	# Has many Item objects
	pass

class ListThing(models.Model):
	list = models.OneToOneField(List)
	
	class Admin:
		pass

class Item(models.Model):
	parent = models.ForeignKey(List)

comment:2 by ttate@…, 19 years ago

From discussion on #django on IRC, it appears that every class needs at least one explicit member data field. Unfortunately, the fact that List "has-many" Item classes doesn't count.

This should probably throw errors in syncdb etc.

comment:3 by James Bennett, 18 years ago

priority: highestnormal

#2209 is a duplicate of this.

comment:4 by James Bennett, 18 years ago

And to reiterate my comment in #2209, it seems that the underlying problem is that syncdb won't create a table in the database for a model with no explicit fields. Thinking about it logically, I believe that's the correct behavior, but it should be documented. Having syncdb error on it is probably a good idea, too.

comment:5 by Tyson Tate <tyson@…>, 18 years ago

I'll start a django-dev thread and perhaps we can get this sucker hashed out. I'd really like to see this fixed up, as it seems that a number of people run in to it. (Also, thanks for setting the status back to normal, I shouldn't have set it high without developer input -- caught up in the moment and whatnot. ;-)

by James Bennett, 18 years ago

Attachment: management.diff added

Patch which makes syncdb raise ImproperlyConfigured on a model with no fields

comment:6 by James Bennett, 18 years ago

Summary: OneToOneField and ForeignKey Crashes Admin[patch] Creating a model with no fields doesn't raise an exception

by James Bennett, 18 years ago

Attachment: management.2.diff added

Better patch; this puts it in model validation

comment:7 by adurdin@…, 18 years ago

How does this patch relate to #2100?

comment:8 by adurdin@…, 18 years ago

Sorry, #2108 -- I misread the number due to the strikethrough.

comment:9 by Russell Keith-Magee, 18 years ago

Cc: freakboy3742@… added

Is this still a problem? I just ran up the test models from this ticket at #2209, and they both seem to happily create the 'empty' tables.

comment:10 by grimboy, 18 years ago

@russellm: I think it only breaks after entering the admin. Personally I'm not sure whether raising an exception is the right decision, but I wasn't on irc when it was discussed and probably missed justification.

comment:11 by Malcolm Tredinnick, 18 years ago

Owner: changed from Adrian Holovaty to Malcolm Tredinnick
Summary: [patch] Creating a model with no fields doesn't raise an exceptionCreating a model with no fields doesn't raise an exception

Although it is possible to get by without models that only have an id field, there are also reasonable arguments for allowing them. It does make some model layouts look a bit nicer on paper.

We already allow the creation and saving of models with only an implicit primary key, so we might as well fix the real bug here: such models won't display in the admin interface, since you can't do anything with them.

Removing the patch keyword because it's not really harmful to create these models and so let's be nice and not make it an error. I'll fix this problem shortly.

comment:12 by Malcolm Tredinnick, 18 years ago

Summary: Creating a model with no fields doesn't raise an exceptionModels with no fields create errors in Admin interface

comment:13 by Gary Wilson <gary.wilson@…>, 18 years ago

Triage Stage: UnreviewedAccepted

comment:14 by Marc Fargas <telenieko@…>, 18 years ago

Has patch: set
Patch needs improvement: set

Marking "has_patch" and "needs_better_patch" as there's patch but a comment says (aprox): We should allow Models without any field except the implicit id field.

Anyway, I tried the test case on from the submitter with latest SVN and I got no errors so maybe we can close this as fixed (dunno when). On the other side I managed to get another bug, I'll file a ticket for it if I don't find one already open ;)

So, somebody please check if you do still have trouble with this specific issue or not.

comment:15 by Jacob, 17 years ago

Resolution: wontfix
Status: newclosed

I really think this is pointless -- editing models with no fields simply doesn't make much sense and seems like a needless corner case to support.

comment:16 by Fabian Topfstedt, 17 years ago

In fact it's not pointless: I have a model with only one field and that has auto_now_add=True, so no field is editable. But if I want to delete an object I need to go to the edit screen first, which is impossible due to this bug.

comment:17 by Derek Anderson <public@…>, 17 years ago

Resolution: wontfix
Status: closedreopened

strongly agree this is not pointless. for instance, i'm writing an app right now and want to keep track of which objects are added together. so i have a "MergeGroup" class with no fields except the implicit id, which my other objects FK to.

comment:18 by Luke Plant, 16 years ago

Resolution: fixed
Status: reopenedclosed

This appears to be fixed after the newforms-admin merge.

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