Opened 15 years ago

Closed 14 years ago

Last modified 12 years ago

#9360 closed (invalid)

Admin interface method get_form does not call get_fieldsets to get fieldsets

Reported by: David Christiansen Owned by: nobody
Component: contrib.admin Version: 1.1
Severity: Keywords: fieldsets form modelform permissions
Cc: domen@… Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I'm presently limiting the fields that some users can edit in our admin interface by defining two fieldsets variables and then overriding get_fieldsets on the admin object to check for a user permission and return the right fieldsets. It looks like this:

def get_fieldsets(self, request, obj=None):
    if request.user.has_perm('page.can_change_structure'):
        return self.fieldsets
    else:
        return self.fieldsets_content_only

However, the exluded fields were being set to blank whenever the form was submitted by a user with fewer privileges. I tracked it down to the get_form method of ModelAdmin. It gets the fieldsets as such: (starting on line 777)

if self.declared_fieldsets:
    fields = flatten_fieldsets(self.declared_fieldsets)
else:
    fields = None

This ignores the get_fieldsets method. I'm not quite sure where the right place to fix this is, so I haven't written a patch. For our use case, it worked to simply override the get_form method and replace the four lines above with:

fields = flatten_fieldsets(self.get_fieldsets(request, obj=obj))

Perhaps something similar would be the right solution overall?

We're running Django 1.0.

Change History (5)

comment:1 by James Bennett, 15 years ago

Resolution: invalid
Status: newclosed

get_fieldsets() is indeed called, at line 517 of contrib/admin/options.py, as part of instantiating the AdminForm (a wrapper around the form which helps with some of the admin niceties). And I've personally got code in production which relies on get_fieldsets() working as advertised, so I know it does.

If your real issue is that you want to change not only the fieldsets but also which fields are made available, you'll need to do a bit more work (e.g., in get_form() to swap out form classes on the fly).

in reply to:  1 comment:2 by Eduardo de Oliveira Padoan, 15 years ago

Replying to ubernostrum:

If your real issue is that you want to change not only the fieldsets but also which fields are made available, you'll need to do a bit more work (e.g., in get_form() to swap out form classes on the fly).

Why not just use get_fieldsets() on get_form() too? If get_formsets() gives the possibility of changing which fields appear to some users, it should be used to determine what gets saved too, IMHO.

comment:3 by Domen Kožar, 14 years ago

Cc: domen@… added
milestone: 1.2
Resolution: invalid
Status: closedreopened
Version: 1.01.1

I have also stumbled upon this issue.

get_fieldsets is not used in get_form, which means it can not be used to populate fieldsets dynamically from request information.

I had to override get_form with following snippet:

        self.fieldsets = self.get_fieldsets(request, obj)

It is not about how much is there to change Django. But the issue is, forward compatibility and how much time user spent for fixing this issue. Took me 2 unproductive hours.

I vote this should be patched.

comment:4 by Russell Keith-Magee, 14 years ago

Resolution: invalid
Status: reopenedclosed

Already closed by a core developer. Start a discussion on django-dev if you disagree.

comment:5 by Jacob, 12 years ago

milestone: 1.2

Milestone 1.2 deleted

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