[Django] #31867: Inconsistency in rendering hidden fields in Django admin
Django
-------------------------------------+-------------------------------------
Reporter: Antoine | Owner: nobody
Humbert |
Type: Bug | Status: new
Component: | Version: 2.1
contrib.admin |
Severity: Normal | Keywords: admin hidden field
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 1 |
-------------------------------------+-------------------------------------
In django admin, when having an inline ModelAdmin with a hidden widget
(e.g. "position" field when using sortable inlines with
django_nested_admin or grappelli), the field may be rendered in admin
interface depending on context:
* when user has change permission on the model, the field is not shown
(because is has an HiddenInput widget)
* when user has view permission, but not change permission:
* If field is in a StackedInline and is the only field on a line (e.g.
fields = ("field1", "field2", "hidden_field") or fields = (("field1",
"field2"), "hidden_field")), then field does not appear in admin interface
* => this is due to the row having the hidden class (coming from
django.contrib.admin.helpers.Fieldline.has_visible_field which is False,
because it is evaluated according to the field widget - which is
HiddenInput). The <div> containing field value itself does not have the
hidden class.
* If field is in a StackedInline and is not the only field on a line
(e.g. fields = ("field1", ("field2", "hidden_field"))), then field appear
in admin interface
* => this time, the row does not have the hidden class, because not
all fields of the line are hidden
* If field is in a TabularInline, then field appear in the admin
interface
* => There is no django.contrig.admin.helpers.Fieldline in this case
which may hide a row containing the field
The inconsistency resided in the fact that
django.contrib.admin.helpers.Fieldline.has_visible_field rely on the field
widget.is_hidden (whatever user has change permission on the model or
nat), whereas in django.contrib.admin.helpers.InlineAdminFormset.fields,
if user has change permission, field is rendered using the field widget
(HiddenInput in this case), but is user does not have change permission,
field widget is statically defined with {'hidden': False}.
In this function, changing lines
{{{
if not self.has_change_permission or field_name in
self.readonly_fields:
yield {
'name': field_name,
'label': meta_labels.get(field_name) or
label_for_field(
field_name,
self.opts.model,
self.opts,
form=empty_form,
),
'widget': {'is_hidden': False},
'required': False,
'help_text': meta_help_texts.get(field_name) or
help_text_for_field(field_name, self.opts.model),
}
}}}
to
{{{
if not self.has_change_permission or field_name in
self.readonly_fields:
yield {
'name': field_name,
'label': meta_labels.get(field_name) or
label_for_field(
field_name,
self.opts.model,
self.opts,
form=empty_form,
),
'widget': {'is_hidden':
empty_form.fields[field_name].widget.is_hidden},
'required': False,
'help_text': meta_help_texts.get(field_name) or
help_text_for_field(field_name, self.opts.model),
}
}}}
effectively hides the field.
I produce the bug in version 2.1, but I expect it to be present is newer
versions as the implied code is the same.
--
Ticket URL: <https://code.djangoproject.com/ticket/31867>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Antoine Humbert | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
Keywords: admin hidden field | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Description changed by Antoine Humbert:
Old description:
New description:
}}}
to
}}}
will hide the table column header for the hidden field. Unfortunatly, it
does not hide the fields values theselves (I'll try to find a workaround
for that)
I produce the bug in version 2.1, but I expect it to be present is newer
versions as the implied code is the same.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/31867#comment:1>
Django
Reporter: Antoine Humbert | Owner: nobody
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Comment (by Antoine Humbert):
To effectively hide fields values, it would be necessary to modify the
django.contrib.admin.helpers.AdminReadonlyField to set the "is_hidden"
property of "field" dictionary, by setting something like this in
constructor :
{{{
self.field = {
'name': class_name,
'label': label,
'help_text': help_text,
'field': field,
}
if field in form.fields:
self.field["is_hidden"] = form.fields[field].widget.is_hidden
}}}
This prevent the values to be shown in the related column. Unfortunately,
the whole field dictionary is displayed at the begining of table row
(where the hidden input widgets would be rendered if user had change
permission). This is the result of mixing dictionary access (for
AdminReadonlyField.field) vs *real* bound field attributes access (for
AdminField.field) in the admin templates.
A solution to prevent the rendering of dictionary at the begining of row,
changing the constructor of AdminReadonlyField with the following works:
{{{
class _FakeField(dict):
def __str__(self):
return ""
self.field = _FakeField(name=class_name, label=label,
help_text=help_text, field=field)
if field in form.fields:
self.field["is_hidden"] = form.fields[field].widget.is_hidden
}}}
Looks like an awefull ack and it may be better to use a simple data-class.
The important thing is that "field.field" in templates must render an
empty string
--
Ticket URL: <https://code.djangoproject.com/ticket/31867#comment:2>
Django
Reporter: Antoine Humbert | Owner: nobody
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Description changed by Antoine Humbert:
Old description:
> In django admin, when having an inline ModelAdmin with a hidden widget
> will hide the table column header for the hidden field. Unfortunatly, it
> does not hide the fields values theselves (I'll try to find a workaround
> for that)
>
> I produce the bug in version 2.1, but I expect it to be present is newer
> versions as the implied code is the same.
New description:
In django admin, when having an inline ModelAdmin with a hidden widget
(e.g. "position" field when using sortable inlines with
django_nested_admin or grappelli), the field may be rendered in admin
interface depending on context:
* when user has change permission on the model, the field is not shown
(because is has an HiddenInput widget)
* when user has view permission, but not change permission:
* If field is in a StackedInline and is the only field on a line (e.g.
fields = ("field1", "field2", "hidden_field") or fields = (("field1",
"field2"), "hidden_field")), then field does not appear in admin interface
* => this is due to the row having the hidden class (coming from
django.contrib.admin.helpers.Fieldline.has_visible_field which is False,
because it is evaluated according to the field widget - which is
HiddenInput). The <div> containing field value itself does not have the
hidden class.
* If field is in a StackedInline and is not the only field on a line
(e.g. fields = ("field1", ("field2", "hidden_field"))), then field appear
in admin interface
* => this time, the row does not have the hidden class, because not
all fields of the line are hidden
* If field is in a TabularInline, then field appear in the admin
interface
* => There is no django.contrig.admin.helpers.Fieldline in this case
which may hide a row containing the field
The inconsistency resides in the fact that
django.contrib.admin.helpers.Fieldline.has_visible_field relies on the
field widget.is_hidden (whatever user has change permission on the model
or not), whereas in
django.contrib.admin.helpers.InlineAdminFormset.fields, if user has change
permission, field is rendered using the field widget (HiddenInput in this
case), but if user does not have change permission, field widget is
}}}
to
}}}
will hide the table column header for the hidden field. Unfortunatly, it
does not hide the fields values theselves (I'll try to find a workaround
for that)
I produce the bug in version 2.1, but I expect it to be present is newer
versions as the implied code is the same.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/31867#comment:3>
Django
Reporter: Antoine Humbert | Owner: nobody
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Description changed by Antoine Humbert:
Old description:
> In django admin, when having an inline ModelAdmin with a hidden widget
> The inconsistency resides in the fact that
> django.contrib.admin.helpers.Fieldline.has_visible_field relies on the
> field widget.is_hidden (whatever user has change permission on the model
> or not), whereas in
> django.contrib.admin.helpers.InlineAdminFormset.fields, if user has
> change permission, field is rendered using the field widget (HiddenInput
> in this case), but if user does not have change permission, field widget
> will hide the table column header for the hidden field. Unfortunatly, it
> does not hide the fields values theselves (I'll try to find a workaround
> for that)
>
> I produce the bug in version 2.1, but I expect it to be present is newer
> versions as the implied code is the same.
New description:
In django admin, when having an inline ModelAdmin with a hidden widget
(e.g. "position" field when using sortable inlines with
django_nested_admin or grappelli), the field may be rendered in admin
interface depending on context:
* when user has change permission on the model, the field is not shown
(because is has an HiddenInput widget)
* when user has view permission, but not change permission:
* If field is in a StackedInline and is the only field on a line (e.g.
fields = ("field1", "field2", "hidden_field") or fields = (("field1",
"field2"), "hidden_field")), then field does not appear in admin interface
* => this is due to the row having the hidden class (coming from
django.contrib.admin.helpers.Fieldline.has_visible_field which is False,
because it is evaluated according to the field widget - which is
HiddenInput). The <div> containing field value itself does not have the
hidden class.
* If field is in a StackedInline and is not the only field on a line
(e.g. fields = ("field1", ("field2", "hidden_field"))), then field appear
in admin interface
* => this time, the row does not have the hidden class, because not
all fields of the line are hidden
* If field is in a TabularInline, then field appear in the admin
interface
* => There is no django.contrig.admin.helpers.Fieldline in this case
which may hide a row containing the field
The inconsistency resides in the fact that
django.contrib.admin.helpers.Fieldline.has_visible_field relies on the
field widget.is_hidden (whatever user has change permission on the model
or not), whereas in
django.contrib.admin.helpers.InlineAdminFormset.fields, if user has change
permission, field is rendered using the field widget (HiddenInput in this
case), but if user does not have change permission, field widget is
}}}
to
}}}
will hide the table column header for the hidden field. Unfortunatly, it
does not hide the fields values themelves (I'll try to find a workaround
for that)
I produce the bug in version 2.1, but I expect it to be present is newer
versions as the implied code is the same.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/31867#comment:4>
Django
------------------------------------+------------------------------------
Reporter: Antoine Humbert | Owner: nobody
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Carlton Gibson):
* has_patch: 1 => 0
* stage: Unreviewed => Accepted
Comment:
Hi Antoine. Thanks for the report — there's certainly an inconsistency
there. The tabular case is easiest to reproduce.
--
Ticket URL: <https://code.djangoproject.com/ticket/31867#comment:5>
Django
------------------------------------+------------------------------------
Reporter: Antoine Humbert | Owner: nobody
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Comment (by Antoine Humbert):
I'll try to provide a PR solving this issue. What should be the starting
commit for the patch ? May I start from the least 2.1 branch or from
develop branch ?
--
Ticket URL: <https://code.djangoproject.com/ticket/31867#comment:6>
Django
Reporter: Antoine Humbert | Owner: Antoine
| Humbert
Type: Bug | Status: assigned
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* owner: nobody => Antoine Humbert
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/31867#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Antoine Humbert | Owner: Antoine
| Humbert
Type: Bug | Status: assigned
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
See [https://github.com/django/django/pull/13299 PR] for master branch
--
Ticket URL: <https://code.djangoproject.com/ticket/31867#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Antoine Humbert | Owner: Antoine
| Humbert
Type: Bug | Status: assigned
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/31867#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Antoine Humbert | Owner: Antoine
| Humbert
Type: Bug | Status: assigned
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* needs_tests: 1 => 0
Comment:
Updates on the PR, unchecking needs tests for another review.
--
Ticket URL: <https://code.djangoproject.com/ticket/31867#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Antoine Humbert | Owner: Antoine
| Humbert
Type: Bug | Status: assigned
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
I've asked for a bit of tidy-up on the PR, but looks OK otherwise. Marking
with ''Patch needs improvement'' — please uncheck that when adjustments
are made so we can take another look. Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/31867#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Antoine Humbert | Owner: Antoine
| Humbert
Type: Bug | Status: assigned
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
Author made requested updates.
--
Ticket URL: <https://code.djangoproject.com/ticket/31867#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: Antoine Humbert | Owner: Antoine
| Humbert
Type: Bug | Status: assigned
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/31867#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: Antoine Humbert | Owner: Antoine
| Humbert
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: fixed
Keywords: admin hidden field | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"de95c826673be9ea519acc86fd898631d1a11356" de95c826]:
{{{
#!CommitTicketReference repository=""
revision="de95c826673be9ea519acc86fd898631d1a11356"
Fixed #31867 -- Made TabularInline handling of hidden fields with view-
only permissions consistent with StackedInline.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31867#comment:14>