Code

Opened 6 years ago

Closed 5 years ago

Last modified 3 years ago

#9362 closed (fixed)

InlineAdminForm breaks content_type FK on model

Reported by: carljm Owned by: carljm
Component: contrib.admin Version: 1.0
Severity: Keywords:
Cc: isometry, airrob Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

r8586 fixed "View on Site" links for admin inline formsets, by having InlineAdminForm.__init__ set a temporary "content_type_id" attribute on the inline-edited "original" instance to the content type id of its model, and then using that attribute in the template to link to the redirection.

This is all fine and dandy, except when the model being edited inline already has a "content_type" field which is a foreign key to ContentType (most likely because it has a generic foreign key). In this case, InlineAdminForm reassigns an already-existing "content_type_id" attribute to point to the wrong model entirely, breaking any behavior of the instance that depends on the generic foreign key.

This behavior can be seen with these test models:

from django.db import models
from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes import generic

class Parent(models.Model):
    name = models.CharField(max_length=50)

    def __unicode__(self):
        return self.name

class Teacher(models.Model):
    name = models.CharField(max_length=50)

    def __unicode__(self):
        return self.name

class Child(models.Model):
    name = models.CharField(max_length=50)
    teacher = models.ForeignKey(Teacher)

    content_type = models.ForeignKey(ContentType)
    object_id = models.PositiveIntegerField()
    parent = generic.GenericForeignKey()

    def __unicode__(self):
        return u'I am %s, a child of %s' % (self.name, self.parent)

With this admin configuration:

from django.contrib import admin
from models import Child, Teacher, Parent

class ChildInline(admin.TabularInline):
    model = Child
    extra = 1

class TeacherAdmin(admin.ModelAdmin):
    inlines = (ChildInline,)

admin.site.register(Teacher, TeacherAdmin)
admin.site.register(Parent)

In the admin, create a Parent named John, then create a Teacher named Sally, then add a Child named Joe using the inline formset for the Teacher. After saving the Child, where the new Child's unicode representation should be displayed as "I am Joe, a child of John", it will instead say "I am Joe, a child of I am Joe, a child of John". This is because InlineAdminForm has changed Joe's content_type_id from Parent to Child, so Joe's "parent" attribute is now pointing to himself.

(The reason for the Teacher model and FK in the above examples is to make it clear that this occurs with any ordinary InlineAdmin, not only with a GenericInlineAdmin).

The easy fix is to use a more obscure attribute name for the temporary "my content type" attribute needed for redirection. Technically it would still be possible to mistakenly overwrite an existing attribute, but much less likely than it is now.

Attachments (2)

9362_r9232.diff (4.4 KB) - added by carljm 6 years ago.
naive fix with regression test
9362_r10178.diff (4.5 KB) - added by carljm 5 years ago.

Download all attachments as: .zip

Change History (14)

Changed 6 years ago by carljm

naive fix with regression test

comment:1 Changed 6 years ago by carljm

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 6 years ago by brosner

  • Owner changed from nobody to brosner
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 6 years ago by isometry

  • Cc isometry added

comment:4 Changed 5 years ago by airrob

  • Cc airrob added

comment:5 Changed 5 years ago by patrickk

is it impolite to ask, whether or not this is going to be fixed ... and when?
the fix seems to be easy and the current behaviour is extremely annoying.

comment:6 Changed 5 years ago by jacob

  • milestone set to 1.1

comment:7 Changed 5 years ago by carljm

  • Owner changed from brosner to carljm
  • Status changed from assigned to new

comment:8 Changed 5 years ago by carljm

Better patch attached after discussion with brosner. Includes test.

Changed 5 years ago by carljm

comment:9 Changed 5 years ago by carljm

  • Status changed from new to assigned

comment:10 Changed 5 years ago by russellm

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

(In [10667]) Fixed #9362 -- Prevented inline forms from overwriting the content_type_id attribute on objets being inlined. Thanks to carljm for the report and patch.

comment:11 Changed 5 years ago by russellm

(In [10671]) [1.0.X] Fixed #9362 -- Prevented inline forms from overwriting the content_type_id attribute on objets being inlined. Thanks to carljm for the report and patch.

Merge of r10667 from trunk.

comment:12 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.