Opened 22 months ago

Closed 8 months ago

#31867 closed Bug (fixed)

Inconsistency rendering hidden fields with view-only permissions in admin

Reported by: Antoine Humbert Owned by: Antoine Humbert
Component: contrib.admin Version: 2.1
Severity: Normal Keywords: admin hidden field
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: yes

Description (last modified by Antoine Humbert)

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 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),
                }

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.

Change History (14)

comment:1 Changed 22 months ago by Antoine Humbert

Description: modified (diff)

comment:2 Changed 22 months ago 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

comment:3 Changed 22 months ago by Antoine Humbert

Description: modified (diff)

comment:4 Changed 22 months ago by Antoine Humbert

Description: modified (diff)

comment:5 Changed 21 months ago by Carlton Gibson

Has patch: unset
Summary: Inconsistency in rendering hidden fields in Django adminInconsistency rendering hidden fields with view-only permissions in admin
Triage Stage: UnreviewedAccepted

Hi Antoine. Thanks for the report — there's certainly an inconsistency there. The tabular case is easiest to reproduce.

If you want to open a PR with your thought, it's probably easier to reason about there.

Last edited 21 months ago by Carlton Gibson (previous) (diff)

comment:6 Changed 21 months ago 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 2.1 branch or from master branch ?

Last edited 21 months ago by Antoine Humbert (previous) (diff)

comment:7 Changed 21 months ago by Antoine Humbert

Owner: changed from nobody to Antoine Humbert
Status: newassigned

comment:8 Changed 21 months ago by Antoine Humbert

Has patch: set

See PR for master branch

comment:9 Changed 21 months ago by Carlton Gibson

Needs tests: set

comment:10 Changed 19 months ago by Carlton Gibson

Needs tests: unset

Updates on the PR, unchecking needs tests for another review.

comment:11 Changed 15 months ago by Carlton Gibson

Patch needs improvement: set

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.

comment:12 Changed 12 months ago by Jacob Walls

Patch needs improvement: unset

Author made requested updates.

comment:13 Changed 9 months ago by Carlton Gibson

Triage Stage: AcceptedReady for checkin

comment:14 Changed 8 months ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In de95c826:

Fixed #31867 -- Made TabularInline handling of hidden fields with view-only permissions consistent with StackedInline.

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