Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#10271 closed (fixed)

Models with multiple inlines inheriting from the same parent class do not save properly in admin

Reported by: Idan Gazit Owned by: Alex Gaynor
Component: contrib.admin Version: 1.0
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I've attached a model graph and screenshots which illustrate the problem a lot more clearly than my words, but bear with me. I've tried my best to make it unconfusing. :)

Basically, I can reproducibly make the admin issue incorrect INSERT statements when saving a model with inlines.

I have a model with inlines of multiple classes (all of which inherit from a common parent). When I create a new instance of the model, it will use only the data from the LAST inline to create all of the model instances for ALL the inlines.

For example: I have two models, TwitterAccount and DeliciousAccount, both of which inherit from Account. Both are displayed inline in the model admin for the Persona model. When creating a new persona, I enter data for a new TwitterAccount and a new DeliciousAccount on the add page for Persona. When I click save, I find that there are (correctly) one new TwitterAccount and one new DeliciousAccount, but (incorrectly) both new accounts are populated with the data I entered for the DeliciousAccount.

I've verified that this behavior only occurs for the pages with the inlines:

  • Model instance creation works as expected from ./manage.py shell
  • Model instance creation works as expected if I create the instances one-by-one from a model-specific admin page and not via inlines

Attached you'll find three PNGs:

  1. 1_admin.png: shows the admin add page for the model with the inlines. Note that the data entered for each inline is different.
  2. 2_sql.png: using the django-debug-toolbar, shows the SQL statements executed after clicking Save. Note that delicious999 is the only data used when creating the inlined model instances. The twitter account is still created — but using the delicious account's data.
  3. model_graph.png: shows the models in the application. The various accounts all inherit from Account, the various activities inherit from Activity.

I'd love to help in fixing this bug but I could use some guidance on what to do next. Thank you!

Attachments (6)

1_admin.png (28.0 KB ) - added by Idan Gazit 16 years ago.
Screenshot of the admin page with data entered
2_sql.png (62.3 KB ) - added by Idan Gazit 16 years ago.
Screenshot of resulting (incorrect) SQL INSERT statements
model_graph.png (77.8 KB ) - added by Idan Gazit 16 years ago.
Application model graph
admin-formset-inheritance.diff (2.7 KB ) - added by Alex Gaynor 16 years ago.
the reporter says this patch fixes it, needs tests
admin-formset-inheritance.2.diff (11.8 KB ) - added by Alex Gaynor 16 years ago.
admin_formset_inheritance.3.diff (15.6 KB ) - added by Idan Gazit 16 years ago.
Same as 2, includes tests

Download all attachments as: .zip

Change History (13)

by Idan Gazit, 16 years ago

Attachment: 1_admin.png added

Screenshot of the admin page with data entered

by Idan Gazit, 16 years ago

Attachment: 2_sql.png added

Screenshot of resulting (incorrect) SQL INSERT statements

by Idan Gazit, 16 years ago

Attachment: model_graph.png added

Application model graph

by Alex Gaynor, 16 years ago

the reporter says this patch fixes it, needs tests

comment:1 by Alex Gaynor, 16 years ago

Has patch: set
Needs tests: set

comment:2 by Alex Gaynor, 16 years ago

It should be noted that all the tests need to be updated to work with this, since it changes the field names.

by Alex Gaynor, 16 years ago

by Idan Gazit, 16 years ago

Same as 2, includes tests

comment:3 by Idan Gazit, 16 years ago

Needs tests: unset

Added tests which checks that distinct data entered on inlines are captured correctly.

This test will work going forward but will throw an error on an unpatched django because (as Alex points out above) the field names have changed. I don't know if this is "good enough" or if I should write more tests -- guidance please!

comment:4 by Alex Gaynor, 16 years ago

Triage Stage: UnreviewedAccepted

Tests all look good and correct by me, needs someone to review and mark as RFC and/or a committer.

comment:5 by Alex Gaynor, 16 years ago

Owner: changed from nobody to Alex Gaynor

comment:6 by Russell Keith-Magee, 16 years ago

Resolution: fixed
Status: newclosed

(In [10017]) Fixed #10271, #10281 -- Fixed the handling multiple inline models that share a common base class and have the link to the inline parent on the base class. Includes modifications that allow the equivalent handling for GenericFields. Thanks to Idan Gazit, Antti Kaihola (akaihola), and Alex Gaynor for their work on this patch.

comment:7 by Russell Keith-Magee, 16 years ago

(In [10019]) [1.0.X] Fixed #10271, #10281 -- Fixed the handling multiple inline models that share a common base class and have the link to the inline parent on the base class. Includes modifications that allow the equivalent handling for GenericFields. Thanks to Idan Gazit, Antti Kaihola (akaihola), and Alex Gaynor for their work on this patch.

Backport of r10017 from trunk.

Note: See TracTickets for help on using tickets.
Back to Top