Fixed #8620 -- Allowed ModelForm.Meta.exclude to exclude explicitely declared fields. #1409

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

Owner

Thanks levity for the report and marcinnowak, himonrura, and koenb
for work on the patch.

I don't see a use case where you would declare a field on a form only to exclude it, but posting a PR to give someone a chance to object.

@timgraham timgraham Fixed #8620 -- Allowed ModelForm.Meta.exclude to exclude explicitely …
…declared fields.

Thanks levity for the report and marcinnowak, himonrura, and koenb
for work on the patch.
506d92c
Member

The design of this concerns me. If exclude is going to apply to explicitly declared fields, then so should fields. But this would mean that you have to add every declared field to fields (or define it as __all__) which would not be good.

I can see it a use case for your scenario - let's suppose you have a field where you want to provide a different user interface (and totally different field with custom processing), but call it the same thing for clarity. It's an edge case but it is feasible.

I actually think solving this ticket would cause some problems. It would mean that if you have a declared field on a superclass of a modelform then you can remove it by defining excludes, but you cannot remove the field for a form (not modelform) if it is defined on a superclass. I don't see why this sort of inheritance should be supported. I think you may be able to resolve the situation in that use case by setting the attribute to be None on the subclass, which is a much less magical way of removing the field. Someone would need to try this out though.

Owner

After discussion on IRC, we came to the consensus that exclude shouldn't be used for advanced cases like this.

@timgraham timgraham closed this Jul 29, 2013
@timgraham timgraham deleted the timgraham:8620 branch Jul 29, 2013
Owner

@timgraham should the ticket also get closed or do you just don't like the pr itself?

Owner

I left the ticket open since the general problem of excluding fields declared in parent forms is still valid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment