Opened 8 years ago

Closed 7 years ago

Last modified 5 years ago

#9122 closed (fixed)

Inline admin on generic relations ignores exclude and max_num

Reported by: Daniel Roseman Owned by: nobody
Component: Contrib apps Version: 1.0
Severity: Keywords: admin, generic relations
Cc: semente@…, Alex Robbins 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 Daniel Roseman 8 years ago.
generic_inline_with_tests.diff (6.6 KB) - added by Alex Robbins 8 years ago.

Download all attachments as: .zip

Change History (11)

Changed 8 years ago by Daniel Roseman

Attachment: generic_inline.diff added

comment:1 Changed 8 years ago by Malcolm Tredinnick

Needs documentation: unset
Needs tests: set
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

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 8 years ago by Guilherme M. Gondim <semente@…>

Cc: semente@… added

comment:3 Changed 8 years ago by Alex Robbins

Cc: Alex Robbins 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 8 years ago by Alex Robbins

comment:4 Changed 8 years ago by Alex Robbins

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 7 years ago by Jacob

milestone: 1.1

comment:6 Changed 7 years ago by Alex Gaynor

Triage Stage: AcceptedReady for checkin

comment:7 Changed 7 years ago by Jacob

Resolution: fixed
Status: newclosed

Fixed in [10586].

comment:8 Changed 7 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 5 years ago by Jacob

milestone: 1.1

Milestone 1.1 deleted

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