Opened 7 years ago

Closed 6 years ago

Last modified 3 years ago

#9122 closed (fixed)

Inline admin on generic relations ignores exclude and max_num

Reported by: danielr Owned by: nobody
Component: Contrib apps Version: 1.0
Severity: Keywords: admin, generic relations
Cc: semente@…, alexr Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

GenericInlineModelAdmin, in contrib.contenttypes.generic, passes most of the usual admin parameters into generic_inlineformset_factory to create the inline formset - but for no obvious reason it omits exclude and max_num, so values for these are ignored.

Patch just adds these, and coerces exclude to a list (rather than a tuple) so that the content_type fields can be automatically appended without breaking.

Attachments (2)

generic_inline.diff (898 bytes) - added by danielr 7 years ago.
generic_inline_with_tests.diff (6.6 KB) - added by alexr 6 years ago.

Download all attachments as: .zip

Change History (11)

Changed 7 years ago by danielr

comment:1 Changed 6 years ago by mtredinnick

  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Before applying this, I'd like to have some tests of that stuff, since it's clear some things have been missed and it would be nice to make sure this doesn't happen in the future (and that the proposed change is correct). It might well turn out to be easiest to replace django/contrib/conttenttypes/tests.py with a django/contrib/contentypes/tests/ directory so that the form tests can be put into a separate file from the default content type tests.

comment:2 Changed 6 years ago by Guilherme M. Gondim <semente@…>

  • Cc semente@… added

comment:3 Changed 6 years ago by alexr

  • Cc alexr added

Ok, I would like to see this bug corrected. I am willing to write the tests but I am not really sure how.

The class we are correcting is GenericInlineModelAdmin, whose constructor has the signature(self, parent_model, admin_site). To test this I guess I have to create an admin site instance to pass to this item. How should I do that? I wondered how the tests worked for the InlineModelAdmin, hoping I could copy them. However, django/contrib/admin doesn't have any tests and a recursive grep of trunk/tests doesn't show anything for InlineModelAdmin.

Any ideas for where to start? (Or tips for other test code that might lead me in the right direction.)

Changed 6 years ago by alexr

comment:4 Changed 6 years ago by alexr

  • Needs tests unset

The attached diff adds on to the regression tests for generic inline models. These tests demonstrate the bug and show it fixed after danielr's patch. They aren't pretty, but I didn't want to add in the complexity of an html parser just to check those elements. That might make the tests too fragile but it seemed like simple was better than accounting for later possibilities.

comment:5 Changed 6 years ago by jacob

  • milestone set to 1.1

comment:6 Changed 6 years ago by Alex

  • Triage Stage changed from Accepted to Ready for checkin

comment:7 Changed 6 years ago by jacob

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

Fixed in [10586].

comment:8 Changed 6 years ago by jacob

(In [10587]) [1.0.X] Fixed #9122: generic inline formsets now respect exclude and max_num. Thanks, Alex Robbins. Backport of [10586] from trunk.

comment:9 Changed 3 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

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