Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 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.

Attachments (0)

Change History (8)

comment:1 Changed 3 years ago by julien

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 Changed 3 years ago by morgan.harris@…

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 Changed 3 years ago by julien

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

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 Changed 3 years ago by anonymous

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 Changed 3 years ago by julien

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 Changed 3 years ago by anonymous

  • Easy pickings unset
  • UI/UX unset

This worked in 1.2.5 and breaks in django 1.3.

comment:7 Changed 3 years ago by aaugustin

This was also reported in #16450.

comment:8 Changed 3 years ago by ramiro

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 3 years ago by ramiro (previous) (diff)

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.