Opened 13 years ago

Closed 13 years ago

Last modified 9 years ago

#8367 closed (fixed)

Small bug in ModelAdmin.get_fieldsets()

Reported by: Owned by: nobody
Component: contrib.admin Version: dev
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


I think this line here:

form = self.get_form(request)

should pass obj to get_form():

form = self.get_form(request, obj)

Change History (7)

comment:1 Changed 13 years ago by Malcolm Tredinnick

You've proposed a solution without describing the problem you're trying to solve. So we can't assess if the solution is appropriate.

The change you mention looks useful, since it allows overriding the form on a per-model basis, but is that the problem that motivated you to file the ticket? If not, what's the real problem at issue here?

comment:2 Changed 13 years ago by

I didn't post the problem because I just discovered this while looking over the code and thought it's trivial :) But here we go:

If I'm thinking straight this could lead to get_form() (overridden) beeing called with obj=None even though there is an object. It could than return a different form which get_fieldsets() (not overriden) would base it's return value on.

I could probably come up with a test case but I think it's enough to argue that if get_form() get's the object it should pass it on to any other function that also takes the object.

comment:3 Changed 13 years ago by

When reading your comment again I figure a simple yes to your question would have been enough :) But anyway I actually ran into the problem today:

    class MyModelAdminForm(forms.ModelForm):
        class Meta:
            model = MyModel
        extra_option = forms.BooleanField()
    class MyModelAdmin(admin.ModelAdmin):
        def get_form(self, request, obj=None, **kwargs):
            if obj:
                return MyModelAdminForm
            return super(MyModelAdmin, self).get_form(request, obj, **kwargs)

This will not display the extra field as get_fieldsets() gets the wrong form when calling get_form().

comment:4 Changed 13 years ago by Brian Rosner

Triage Stage: UnreviewedAccepted

comment:5 Changed 13 years ago by Yuri Baburov

duplicate of #7703 and #8400

comment:6 Changed 13 years ago by Brian Rosner

Resolution: fixed
Status: newclosed

(In [8479]) Fixed #8367 -- Pass the object to get_form from get_fieldsets in ModelAdmin. Thanks jarrow for catching this.

comment:7 Changed 9 years ago by Jacob

milestone: 1.0

Milestone 1.0 deleted

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