Opened 4 years ago

Closed 3 years 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 by Antoine Humbert, 4 years ago

Description: modified (diff)

comment:2 by Antoine Humbert, 4 years ago

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 by Antoine Humbert, 4 years ago

Description: modified (diff)

comment:4 by Antoine Humbert, 4 years ago

Description: modified (diff)

comment:5 by Carlton Gibson, 4 years ago

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 4 years ago by Carlton Gibson (previous) (diff)

comment:6 by Antoine Humbert, 4 years ago

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 4 years ago by Antoine Humbert (previous) (diff)

comment:7 by Antoine Humbert, 4 years ago

Owner: changed from nobody to Antoine Humbert
Status: newassigned

comment:8 by Antoine Humbert, 4 years ago

Has patch: set

See PR for master branch

comment:9 by Carlton Gibson, 4 years ago

Needs tests: set

comment:10 by Carlton Gibson, 4 years ago

Needs tests: unset

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

comment:11 by Carlton Gibson, 4 years ago

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

Patch needs improvement: unset

Author made requested updates.

comment:13 by Carlton Gibson, 3 years ago

Triage Stage: AcceptedReady for checkin

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

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