[Django] #29487: Admin "read only" doesn't check change permission for object specifically
Django
-------------------------------------------+------------------------
Reporter: Matthew Frazier | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------------+------------------------
When determining which fields are read-only, `ModelAdmin._changeform_view`
calls `ModelAdmin.has_change_permission(request)` without the object
parameter - so, even if the user has read-only permission, only fields
returned by `get_readonly_fields()` are included. However, subsequent
calls to `has_change_permission` to build the form do use the object
parameter - so, all fields are left out of the form, with the assumption
that they are included in `get_readonly_fields()`. This leads to a
traceback when rendering the template, since none of the fields are
present in the form. (The traceback is included below.)
Changing `has_change_permission(request)` to
`has_change_permission(request, obj)` on
[[https://github.com/django/django/blob/860903b261517442098bdf14e44f4fd4ae20aeb9/django/contrib/admin/options.py#L1580|line
1580 of django/contrib/admin/options.py]] resolves the issue.
{{{
Request Method: GET
Request URL: http://localhost:8000/admin/.../.../.../change/
Django Version: 2.1a1
Python Version: 3.6.5
Template error:
In template
.../django/contrib/admin/templates/admin/includes/fieldset.html, error at
line 7
Key 'full_name' not found in 'UserForm'. Choices are: .
1 : <fieldset class="module aligned {{ fieldset.classes }}">
2 : {% if fieldset.name %}<h2>{{ fieldset.name }}</h2>{% endif %}
3 : {% if fieldset.description %}
4 : <div class="description">{{ fieldset.description|safe
}}</div>
5 : {% endif %}
6 : {% for line in fieldset %}
7 : <div class="form-row{% if line.fields|length_is:'1' and
line.errors %} errors{% endif %}{% if not line.has_visible_field %}
hidden{% endif %} {% for field in line %} {% if field.field.name %}
field-{{ field.field.name }}{% endif %}{% endfor %}">
8 : {% if line.fields|length_is:'1' %}{{ line.errors }}{%
endif %}
9 : {% for field in line %}
10 : <div{% if not line.fields|length_is:'1' %}
class="fieldBox{% if field.field.name %} field-{{ field.field.name }}{%
endif %}{% if not field.is_readonly and field.errors %} errors{% endif
%}{% if field.field.is_hidden %} hidden{% endif %}"{% elif
field.is_checkbox %} class="checkbox-row"{% endif %}>
11 : {% if not line.fields|length_is:'1' and not
field.is_readonly %}{{ field.errors }}{% endif %}
12 : {% if field.is_checkbox %}
13 : {{ field.field }}{{ field.label_tag }}
14 : {% else %}
15 : {{ field.label_tag }}
16 : {% if field.is_readonly %}
17 : <div class="readonly">{{
field.contents }}</div>
Traceback:
File ".../django/forms/forms.py" in __getitem__
163. field = self.fields[name]
During handling of the above exception ('full_name'), another exception
occurred:
...
File ".../django/template/base.py" in render_annotated
904. return self.render(context)
File ".../django/template/defaulttags.py" in render
165. values = list(values)
File ".../django/contrib/admin/helpers.py" in __iter__
118. yield AdminField(self.form, field, is_first=(i ==
0))
File ".../django/contrib/admin/helpers.py" in __init__
130. self.field = form[field] # A django.forms.BoundField
instance
File ".../django/forms/forms.py" in __getitem__
169. ', '.join(sorted(f for f in self.fields)),
Exception Type: KeyError
Exception Value: "Key 'full_name' not found in 'UserForm'. Choices are: ."
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29487>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Matthew Frazier | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Tim Graham):
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/29487#comment:1>
Django
Reporter: Matthew Frazier | Owner: Paulo
Type: Bug | Status: assigned
Component: contrib.admin | Version: 2.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Paulo):
* owner: nobody => Paulo
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/29487#comment:2>
Django
Reporter: Matthew Frazier | Owner: Paulo
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Carlton Gibson):
* cc: Carlton Gibson (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/29487#comment:3>
Django
Reporter: Matthew Frazier | Owner: Paulo
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Tim Graham):
Paulo, are you able to complete this before Monday's beta release?
--
Ticket URL: <https://code.djangoproject.com/ticket/29487#comment:4>
Django
Reporter: Matthew Frazier | Owner: Paulo
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Paulo):
I can prepare it today or tomorrow latest. Would that work for you?
My reply keeps getting marked as spam :/
--
Ticket URL: <https://code.djangoproject.com/ticket/29487#comment:5>
Django
Reporter: Matthew Frazier | Owner: Paulo
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Paulo):
PR: https://github.com/django/django/pull/10055
--
Ticket URL: <https://code.djangoproject.com/ticket/29487#comment:6>
Django
Reporter: Matthew Frazier | Owner: Paulo
Component: contrib.admin | Version: 2.1
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"553617e61324dd5d9b34c47ceb2b6f20888daf20" 553617e6]:
{{{
#!CommitTicketReference repository=""
revision="553617e61324dd5d9b34c47ceb2b6f20888daf20"
Fixed #29487 -- Accounted for object level permissions when calculating
change view's read-only fields.
Thanks Matthew Frazier for the report and fix.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29487#comment:7>
Django
Reporter: Matthew Frazier | Owner: Paulo
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"8cbfaf29c06ffc8b81c12ef30e0d151cae2d9712" 8cbfaf29]:
{{{
#!CommitTicketReference repository=""
revision="8cbfaf29c06ffc8b81c12ef30e0d151cae2d9712"
[2.1.x] Fixed #29487 -- Accounted for object level permissions when
calculating change view's read-only fields.
Thanks Matthew Frazier for the report and fix.
Backport of 553617e61324dd5d9b34c47ceb2b6f20888daf20 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29487#comment:8>