Code

Opened 8 years ago

Closed 7 years ago

#1939 closed defect (wontfix)

[patch] edit_inline and Multiple ForeignKeys to same model breaks the admin

Reported by: ww@… Owned by: nobody
Component: contrib.admin Version: master
Severity: critical Keywords:
Cc: gary.wilson@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by adrian)

A minimal model that reproduces the problem:

class Account(models.Model):
	class Admin:
		pass
	owner = models.CharField(maxlength = 32)
	
class Transaction(models.Model):
	account = models.ForeignKey(Account, core = True, related_name = 'transactions1',
				    edit_inline = models.TABULAR,
				    num_in_admin = 1)
	contra = models.ForeignKey(Account, related_name = 'transactions2')
	description = models.CharField(maxlength = 64)

This code, when run in the admin, and when goes to the Account add screen, produces
the following traceback:

Traceback (most recent call last):
File "/home/ww/django_src/django/template/__init__.py" in render_node
  701. result = node.render(context)
File "/home/ww/django_src/django/template/defaulttags.py" in render
  113. nodelist.append(node.render(context))
File "/home/ww/django_src/django/contrib/admin/templatetags/admin_modify.py" in render
  155. bound_related_object = relation.bind(context['form'], original, bound_related_object_class)
File "/home/ww/django_src/django/db/models/related.py" in bind
  122. return bound_related_object_class(self, field_mapping, original)
File "/home/ww/django_src/django/contrib/admin/templatetags/admin_modify.py" in __init__
  136. self.form_field_collection_wrappers = [FormFieldCollectionWrapper(field_mapping ,fields, i)
File "/home/ww/django_src/django/contrib/admin/templatetags/admin_modify.py" in __init__
  112. self.bound_fields = [AdminBoundField(field, self.field_mapping, field_mapping['original'])
File "/home/ww/django_src/django/contrib/admin/views/main.py" in __init__
  115. self.form_fields = [field_mapping[name] for name in self.field.get_manipulator_field_names('')]
File "/home/ww/django_src/django/forms/__init__.py" in __getitem__
  195. return self.formfield_dict[template_key]

  KeyError at /admin/record/account/add/
  'account'

I am running SVN as of a few minutes ago.

Attachments (2)

inline_multiple_foreign.diff (4.4 KB) - added by Joel Heenan <joelh-django@…> 8 years ago.
patch for ticket 1939
inline_multiple_foreign2.diff (665 bytes) - added by joe@… 7 years ago.
simpler patch

Download all attachments as: .zip

Change History (13)

comment:1 Changed 8 years ago by [530]

Cleaned up the model code

class Account(models.Model):

    owner = models.CharField(maxlength = 32)

    class Admin:
        pass
    
    class Transaction(models.Model):

        account = models.ForeignKey(Account, core = True, related_name = 'transactions1', edit_inline = models.TABULAR, num_in_admin = 1)
        contra = models.ForeignKey(Account, related_name = 'transactions2')
        description = models.CharField(maxlength = 64)

    class Admin:
        pass


comment:2 Changed 8 years ago by adrian

  • Description modified (diff)

Fixed formatting in description.

comment:3 Changed 8 years ago by Gary Wilson <gary.wilson@…>

  • Cc gary.wilson@… added
  • Summary changed from Multiple Foreign Keys breaks the admin to edit_inline and Multiple ForeignKeys to same model breaks the admin

Did a little testing, and it seems that an error only occurs when using edit_inline and multiple ForeignKeys to the same model.

I do not see this error with:

  • Two ForeignKeys to the same model and no edit_inline.
  • Two ForeignKeys to two different models, with or without edit_inline.

comment:4 Changed 8 years ago by Joel Heenan <joelh-django@…>

The admin interface is full of some nasty hacks. I wouldn't be surprised if it makes an assumption that there will only be one inline foreign key object per model. The MultiValueDict submitted into the manipulator appears to have keys in the form "<lowercase_foreign_model_name>.<sequence_number>.<member>". Somewhere in that you probably want to have the local member name of the foreign key. I have no idea exactly what relies on the key being in this weird format though.

Changed 8 years ago by Joel Heenan <joelh-django@…>

patch for ticket 1939

comment:5 Changed 8 years ago by Joel Heenan <joelh-django@…>

I've attached a patch that begins to address this issue. There is still a lot more work to be done to get edit_inline working perfectly smoothly, I'm curious to see how it survives with 3 foreign keys.

The reason for the initial traceback was contrib/admin/views/main.py does not check that followed fields should be edited inline. I also found a few areas in the manipulator where it gets a bit confused between two foreign keys to the one table. Finally the Options class flatten_data again doesn't check whether a the relation is edited inline - this causes the dict returned by InlineObjectCollection to get overwritten by Transaction records that are not to be edited inline anyway. It seems bad to me that the flatten_data format does not contain the foreign key name.

Note to get this to work you probably want blank=True,null=True on the contra foreign key. Otherwise it will be impossible to add the first account.

comment:6 Changed 8 years ago by grimboy

  • Summary changed from edit_inline and Multiple ForeignKeys to same model breaks the admin to [patch] edit_inline and Multiple ForeignKeys to same model breaks the admin

comment:7 Changed 7 years ago by Gary Wilson <gary.wilson@…>

#3305, #2522, and #1577 marked as duplicates of this.

comment:8 Changed 7 years ago by Gary Wilson <gary.wilson@…>

  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

comment:9 Changed 7 years ago by Nicola Larosa <nico@…>

#4937 marked as duplicate of this.

comment:10 Changed 7 years ago by joe@…

In django 0.96, I'm getting a different exception in this case:

Traceback (most recent call last):
File "/var/lib/python-support/python2.5/django/template/__init__.py" in render_node
  723. result = node.render(context)
File "/var/lib/python-support/python2.5/django/template/defaulttags.py" in render
  122. nodelist.append(node.render(context))
File "/var/lib/python-support/python2.5/django/contrib/admin/templatetags/admin_modify.py" in render
  170. bound_related_object = relation.bind(context['form'], original, bound_related_object_class)
File "/var/lib/python-support/python2.5/django/db/models/related.py" in bind
  129. return bound_related_object_class(self, field_mapping, original)

  TypeError at /admin/polls/poll/2/
  'bool' object is not callable

The direct reason is this (django/contrib/admin/templatetags/admin_modify.py, line 163-168):

if relation.field.rel.edit_inline == models.TABULAR:
  bound_related_object_class = TabularBoundRelatedObject
elif relation.field.rel.edit_inline == models.STACKED:
  bound_related_object_class = StackedBoundRelatedObject
else:
  bound_related_object_class = relation.field.rel.edit_inline

When called on an object without edit_inline set, "edit_inline" is implicitly False, so bound_related_object_class is set to False, which is not callable.

I've attached a much less invasive patch, which takes the fields without edit_inline out of the related fields list *only* when that list is being passed to the admin template to be rendered inline. I don't know how this interacts with the first patch, but I prefer mine because the first patch strikes me as dangerous - it takes edit_inline into account at too low a level.

Changed 7 years ago by joe@…

simpler patch

comment:11 Changed 7 years ago by ubernostrum

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

edit_inline is being completely rewritten for newforms-admin, so this is no longer valid.

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.