Code

Opened 6 years ago

Closed 5 years ago

Last modified 3 years ago

#8483 closed (wontfix)

Admin interface does not check for incompatible generic relations

Reported by: smoluf Owned by: ramiro
Component: contrib.admin Version: master
Severity: Keywords: pycamp2009
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

# myapp.models.py
from django.db import models
from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes import generic

class User (models.Model):
    username = models.CharField(max_length=50, primary_key=True)
    birthdate = models.DateField()

class PhoneNumber:
    phone_number = PhoneNumberField()
    content_type = models.ForeignKey(ContentType)
    object_id = models.PositiveIntegerField()
    content_object = generic.GenericForeignKey()
# myapp.admin.py
from django.contrib import admin
from myapp.models import *
from django.contrib.contenttypes import generic

class PhoneNumberInline (admin.TabularInline):
    model = PhoneNumber

class UserAdmin (admin.ModelAdmin):
    inlines = [
        PhoneNumberInline,
    ]
admin.site.register(User, UserAdmin)

In the above example, a PhoneNumber cannot be associated with a User because the PhoneNumber stores object_id as a PositiveIntegerField, whereas the primary key of User is a CharField. However, the admin interface does not check for this and throws a ValueError exception when it attempts to give PhoneNumber.object_id (an integer) the value of User.username (a string).

It seems to me this should be a ConfigurationError.

Attachments (1)

8483-generic_fk_validations.diff (2.8 KB) - added by ramiro 5 years ago.
Ptahc that adds the validation for the reported user error and a couple of related ones

Download all attachments as: .zip

Change History (7)

comment:1 Changed 6 years ago by kratorius

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Version changed from 1.0-beta-1 to SVN

This seems related to #8316.

comment:2 Changed 5 years ago by jacob

  • milestone set to 1.1
  • Triage Stage changed from Unreviewed to Accepted

Changed 5 years ago by ramiro

Ptahc that adds the validation for the reported user error and a couple of related ones

comment:3 Changed 5 years ago by ramiro

  • Keywords pycamp2009 added
  • Owner changed from nobody to ramiro

comment:4 Changed 5 years ago by fperetti

  • Has patch set

comment:5 Changed 5 years ago by jacob

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

It turns out that this is nearly impossible to check accurately. For example, the check in your patch for the class of the object_id field against the model's PK can't take into account fields that can be coerced into the right type. This is especially a problem because AutoField is one of those -- it's not an IntegerField, but it has a correctly typed value. Your check, then, prevents an IntegerField object_id from pointing to an AutoField, which is clearly incorrect. But it's more subtle than that: think about things like FilePathField that don't look anything like a CharField, but end up returning characters.

Ultimately, this is just a case where we have to expect users to do the right thing. We can't check every possible mistake someone might make.

comment:6 Changed 3 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

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.