Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#10271 closed (fixed)

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

Reported by: idangazit Owned by: Alex
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: UI/UX:

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 idangazit 7 years ago.
Screenshot of the admin page with data entered
2_sql.png (62.3 KB) - added by idangazit 7 years ago.
Screenshot of resulting (incorrect) SQL INSERT statements
model_graph.png (77.8 KB) - added by idangazit 7 years ago.
Application model graph
admin-formset-inheritance.diff (2.7 KB) - added by Alex 7 years ago.
the reporter says this patch fixes it, needs tests
admin-formset-inheritance.2.diff (11.8 KB) - added by Alex 7 years ago.
admin_formset_inheritance.3.diff (15.6 KB) - added by idangazit 7 years ago.
Same as 2, includes tests

Download all attachments as: .zip

Change History (13)

Changed 7 years ago by idangazit

Screenshot of the admin page with data entered

Changed 7 years ago by idangazit

Screenshot of resulting (incorrect) SQL INSERT statements

Changed 7 years ago by idangazit

Application model graph

Changed 7 years ago by Alex

the reporter says this patch fixes it, needs tests

comment:1 Changed 7 years ago by Alex

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

comment:2 Changed 7 years ago by Alex

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

Changed 7 years ago by Alex

Changed 7 years ago by idangazit

Same as 2, includes tests

comment:3 Changed 7 years ago by idangazit

  • 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 Changed 7 years ago by Alex

  • Triage Stage changed from Unreviewed to Accepted

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

comment:5 Changed 7 years ago by Alex

  • Owner changed from nobody to Alex

comment:6 Changed 6 years ago by russellm

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

(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 Changed 6 years ago by russellm

(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