Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#15771 closed Bug (wontfix)

"from django.contrib.auth.admin import UserAdmin" breaks backwards relations for User

Reported by: morgan.harris@… Owned by: nobody
Component: contrib.auth Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Importing the model UserAdmin from the package django.contrib.auth.admin breaks any backwards relations that exist on User. An example models.py:

from django.db import models
from django.contrib.auth.models import User
from django.contrib.auth.admin import UserAdmin

# Create your models here.

class UserProfile(models.Model):
	"""Represents extra information on a user in the system"""
	
	user = models.OneToOneField(User, related_name='profile')
	is_staff = models.BooleanField(default=False)
	is_external = models.BooleanField(default=False)
	faculty = models.CharField(max_length=255,blank=True)

and an example of the error in practice:

# python manage.py shell

Python 2.7 (r27:82508, Jul  3 2010, 21:12:11) 
[GCC 4.0.1 (Apple Inc. build 5493)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from usertest.users.models import *
>>> User.objects.filter(profile__faculty='Engineering')
Traceback (most recent call last):
 [...]
FieldError: Cannot resolve keyword 'profile' into field. Choices are: _message_set, date_joined, email, first_name, groups, id, is_active, is_staff, is_superuser, last_login, last_name, password, user_permissions, username

If the class isn't imported, the result is thus:

# python manage.py shell

Python 2.7 (r27:82508, Jul  3 2010, 21:12:11) 
[GCC 4.0.1 (Apple Inc. build 5493)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from usertest.users.models import *
>>> User.objects.filter(profile__faculty='Engineering')
[]

This bug is new as of version 1.3.

Change History (8)

comment:1 by Julien Phalip, 13 years ago

It is not very good practice to import admin business inside a models.py, both conceptually (if we care about MVC) and technically (potential circular imports). Is that something that used to work, and if so, in which version of Django?

comment:2 by morgan.harris@…, 13 years ago

Worked in 1.2.1. Was extending it to make our admin interface a bit friendlier, but not wholly necessary now that we import our data from an LDAP directory. Figured if something used to work and doesn't now that's probably a bug.

comment:3 by Julien Phalip, 13 years ago

Resolution: wontfix
Status: newclosed

The admin import system is quite convoluted and, like I've pointed out above, importing admin stuff into a models.py seems like not so good practice in the first place anyway. On that basis I'm wontfixing this. Please reopen if you have a strong use-case that cannot be filled using a better approach.

comment:4 by anonymous, 13 years ago

The underlying problem here is likely the same as #11247/#11448, it has been around for a long time. Possibly something has changed in the contrib.auth code to trigger it now where it wasn't triggered in the past. While it is true that intermixing model and admin definitions is generally bad practice, getting the underlying bug fixed would be a good idea. It's not particularly nice that a harmless-looking sequence of imports (even if they are not best practice) would cause this kind of side-effect.

comment:5 by Julien Phalip, 13 years ago

Thanks for this info. I agree that it'd be nice to solve this bug, which #11448 appears to be addressing at the root. In the particular instance here, my point was that in order to work the admin has to do some pretty convoluted imports, leading to some inevitable limitations. For example, the recommended place to do autodiscover() is in the main urls.py because it is assumed that by the time that module gets loaded all models have already been imported. So the admin already does prevent some things that you may like to do, and if those things aren't best practice, then it's something that we can live with. But yeah, we should fix this bug if we can (via #11448).

comment:6 by anonymous, 13 years ago

Easy pickings: unset
UI/UX: unset

This worked in 1.2.5 and breaks in django 1.3.

comment:7 by Aymeric Augustin, 13 years ago

This was also reported in #16450.

comment:8 by Ramiro Morales, 13 years ago

Can you please try replacing Django 1.3 with a fresh checkout of the bugfixes-ony 1.3.X SVN branch and repating your test? I'm particularly interested in knowing if this could have been solved by r16541. Thanks.

Last edited 13 years ago by Ramiro Morales (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top