#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: | no | UI/UX: | no |
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)
Change History (11)
by , 16 years ago
| Attachment: | generic_inline.diff added |
|---|
comment:1 by , 16 years ago
| Needs tests: | set |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:2 by , 16 years ago
| Cc: | added |
|---|
comment:3 by , 16 years ago
| Cc: | 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.)
by , 16 years ago
| Attachment: | generic_inline_with_tests.diff added |
|---|
comment:4 by , 16 years ago
| 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 by , 16 years ago
| milestone: | → 1.1 |
|---|
comment:6 by , 16 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
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.pywith adjango/contrib/contentypes/tests/directory so that the form tests can be put into a separate file from the default content type tests.