#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)
follow-up: 2 comment:1 by , 16 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:2 by , 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 , 15 years ago
Cc: | added |
---|---|
milestone: | → 1.2 |
Resolution: | invalid |
Status: | closed → reopened |
Version: | 1.0 → 1.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 , 15 years ago
Resolution: | → invalid |
---|---|
Status: | reopened → closed |
Already closed by a core developer. Start a discussion on django-dev if you disagree.
get_fieldsets()
is indeed called, at line 517 ofcontrib/admin/options.py
, as part of instantiating theAdminForm
(a wrapper around the form which helps with some of the admin niceties). And I've personally got code in production which relies onget_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).