Code

Opened 8 years ago

Closed 6 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: master
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: UI/UX:

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 ubernostrum 8 years ago.
Patch which makes syncdb raise ImproperlyConfigured on a model with no fields
management.2.diff (722 bytes) - added by ubernostrum 8 years ago.
Better patch; this puts it in model validation

Download all attachments as: .zip

Change History (20)

comment:1 Changed 8 years ago by ttate@…

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 Changed 8 years ago by ttate@…

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 Changed 8 years ago by ubernostrum

  • priority changed from highest to normal

#2209 is a duplicate of this.

comment:4 Changed 8 years ago by ubernostrum

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 Changed 8 years ago by Tyson Tate <tyson@…>

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. ;-)

Changed 8 years ago by ubernostrum

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

comment:6 Changed 8 years ago by ubernostrum

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

Changed 8 years ago by ubernostrum

Better patch; this puts it in model validation

comment:7 Changed 8 years ago by adurdin@…

How does this patch relate to #2100?

comment:8 Changed 8 years ago by adurdin@…

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

comment:9 Changed 8 years ago by russellm

  • 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 Changed 8 years ago by grimboy

@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 Changed 8 years ago by mtredinnick

  • Owner changed from adrian to mtredinnick
  • Summary changed from [patch] Creating a model with no fields doesn't raise an exception to Creating 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 Changed 8 years ago by mtredinnick

  • Summary changed from Creating a model with no fields doesn't raise an exception to Models with no fields create errors in Admin interface

comment:13 Changed 7 years ago by Gary Wilson <gary.wilson@…>

  • Triage Stage changed from Unreviewed to Accepted

comment:14 Changed 7 years ago by Marc Fargas <telenieko@…>

  • 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 Changed 6 years ago by jacob

  • Resolution set to wontfix
  • Status changed from new to closed

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 Changed 6 years ago by fabian

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 Changed 6 years ago by Derek Anderson <public@…>

  • Resolution wontfix deleted
  • Status changed from closed to reopened

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 Changed 6 years ago by lukeplant

  • Resolution set to fixed
  • Status changed from reopened to closed

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.