Code

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#9498 closed (fixed)

generic inline formsets: object has no attribute 'fk'

Reported by: fredbartle Owned by: brosner
Component: Contrib apps Version: master
Severity: Keywords: generic inline formsets
Cc: carljm, benspaulding Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

As of r9297, which django.contrib.admin.helpers now utilizes formset.fk, generic inline formsets now break attempting to access formset.fk. Attached is a simple fix, though I suspect there's more work to be done with generic and r9297.

Attachments (4)

generic.py.r9329.diff (504 bytes) - added by fredbartle 5 years ago.
generic.py.r9329_2.diff (728 bytes) - added by markus _forgot_ evigo _spam_ net 5 years ago.
generic.py_helpers.py_r9329.diff (892 bytes) - added by markus _uploading_ evigo _patch_ net 5 years ago.
generic_inline_tests.diff (6.1 KB) - added by danielr 5 years ago.

Download all attachments as: .zip

Change History (14)

Changed 5 years ago by fredbartle

comment:1 Changed 5 years ago by fredbartle

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

comment:2 Changed 5 years ago by fredbartle

Here's a basic example:

class Attribute(models.Model):
    content_type = models.ForeignKey(ContentType)
    object_id = models.PositiveIntegerField()
    content_object = generic.GenericForeignKey('content_type', 'object_id')
    key = models.CharField(max_length=30)
    value = models.CharField(max_length=30)

class MyModel(models.Model):
    name = models.CharField(max_length=30)
    attributes = generic.GenericRelation(Attribute)


class AttributeInline(generic.GenericTabularInline):
    model = Attribute

class MyModelAdmin(models.Model):
    inlines = [AttributeInline]

comment:3 Changed 5 years ago by brosner

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

comment:4 Changed 5 years ago by markus@…

applying the patch helped, but directly showed a new bug:

http://nopaste.com/p/aJeKlSyvj

looking into contrib/contenttypes/generic.py I found this:

if exclude is not None: 
    exclude.extend([ct_field.name, fk_field.name])
else: 
    exclude = [ct_field.name, fk_field.name]

makes no sense to me, but i'm not the architect ;)
So i'm attaching another patch, which includes the first and
deletes the whole "else" - for me this fixes GenericTabularInline

Changed 5 years ago by markus _forgot_ evigo _spam_ net

comment:5 Changed 5 years ago by markus _presents_a_better_ evigo _solution_ net

so once again:

after some testing i've seen, that those lines:

if exclude is not None:
    exclude.extend({ct_field.name, fk_field.name})
else:
    exclude = [ct_field.name, fk_field.name]

Are indeed _not_ useless! It's absolutly clear (now - for me - finally), that
we of course don't need the ForeignKey in a GenericInlineFormSet, because it's
clear that we set the ForeignKey according the Model we are "inlining". So without
the "else" from the snippet, I always had a content_type and object_id field in
my inline-form, what seems to be a new-self-produced-bug - means: re-add those two
lines and restart searching, why I always get: http://nopaste.com/p/aJeKlSyvj

The problem with this is, the fk_field is beeing called directly from the template,
so there is no way to avoid beeing asked for the fk_field-widget. As mentioned before,
we have absolutly no need for the fk-field, as it gets set automaticly by the parent
model. So i just looked for the output-method for fk_field - in contrib/admin/helpers.py:

def fk_field(self):
        return AdminField(self.form, self.formset.fk.name, False)

inside "InlineAdminForm", means for me only inlined-models come across this form,
so it's painless to just change it to:

def fk_field(self):
    try:
        return AdminField(self.form, self.formset.fk.name, False)
    except KeyError, e:
        return ""

This actually is just a silent failure, which still works if needed.
(Isn't this anyway planned/recommended for template-parsing?)

So there is another file attached, including this and the first patch.
Do _NOT_ use the second patch, it's more an ugly hack than a solution ;)

hope i could help,
greets

Changed 5 years ago by markus _uploading_ evigo _patch_ net

comment:6 Changed 5 years ago by carljm

  • Cc carljm added

comment:7 Changed 5 years ago by benspaulding

  • Cc benspaulding added

Changed 5 years ago by danielr

comment:8 Changed 5 years ago by danielr

  • Needs tests unset

Patch that adds regression tests for this problem.

comment:9 Changed 5 years ago by brosner

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

(In [9413]) [1.0.X] Fixed #9498 -- Handle a formset correctly when the foreign key is not available (for now).

This case pops up with generic foreign key inlines after [9297]. Added tests
to handle future regressions with generic foreign key inlines in the admin.

Thanks markus and danielr for patches.

Backport of [9412] from trunk.

comment:10 Changed 5 years ago by brosner

This was also applied on trunk in [9412] -- dang post-commit hook :)

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.