Opened 16 years ago

Closed 16 years ago

Last modified 12 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

Description

I think this line here:

form = self.get_form(request)

should pass obj to get_form():

form = self.get_form(request, obj)

http://code.djangoproject.com/browser/django/trunk/django/contrib/admin/options.py?rev=8396#L334

Change History (7)

comment:1 by Malcolm Tredinnick, 16 years ago

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 by , 16 years ago

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 by , 16 years ago

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 by Brian Rosner, 16 years ago

Triage Stage: UnreviewedAccepted

comment:5 by Yuri Baburov, 16 years ago

duplicate of #7703 and #8400

comment:6 by Brian Rosner, 16 years ago

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

milestone: 1.0

Milestone 1.0 deleted

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