[Django] #29969: Admin inline with view permission is shown when save_as validation fails
Django
-----------------------------------------+------------------------
Reporter: matijakolaric | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 2.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
How to reproduce:
Make a modeladmin with save_as=True, and one unique field, with an inline
where the user only has 'view' permission.
Create an instance. Open it, press "save as new". This will result in the
validation error, as it should, but the inline is shown as editable with 3
extra fields (ignoring the actual extra setting).
If the unique field is changed, even if the data is entered in the inline
form, nothing is saved (so there is no security issue, it just looks bad).
--
Ticket URL: <https://code.djangoproject.com/ticket/29969>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: matijakolaric | Owner: nobody
Type: Uncategorized | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by matijakolaric):
* component: Uncategorized => contrib.admin
--
Ticket URL: <https://code.djangoproject.com/ticket/29969#comment:1>
Django
Reporter: matijakolaric | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* type: Uncategorized => Bug
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/29969#comment:2>
Django
Reporter: matijakolaric | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by matijakolaric:
Old description:
> How to reproduce:
>
> Make a modeladmin with save_as=True, and one unique field, with an inline
> where the user only has 'view' permission.
>
> Create an instance. Open it, press "save as new". This will result in the
> validation error, as it should, but the inline is shown as editable with
> 3 extra fields (ignoring the actual extra setting).
>
> If the unique field is changed, even if the data is entered in the inline
> form, nothing is saved (so there is no security issue, it just looks
> bad).
New description:
How to reproduce:
Make a modeladmin with save_as=True, and one unique field, with an inline
where the user only has 'view' permission.
Create an instance. Open it, press "save as new".
This will result in the validation error, as it should, but the inline is
shown as editable with empty forms. The number of forms corresponds to the
number of inline forms.
If the unique field is changed, even if the data is entered in the inline
form, nothing is saved (so there is no security issue, it just looks bad).
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29969#comment:3>
Django
Reporter: matijakolaric | Owner: Carlton
| Gibson
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: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* owner: nobody => Carlton Gibson
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/29969#comment:4>
Django
Reporter: Matija Kolarić | Owner: Carlton
| Gibson
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: 0 | UI/UX: 0
Comment (by Carlton Gibson):
Fix for #29930 ([https://github.com/django/django/pull/10678 PR] would
"solve" this, in that the inline formset is removed when re-presenting the
parent form with the validation error.
Question is whether that's good enough for this corner case? Or do we want
to represent the read-only inline? (2nd option is better; haven't yet got
to seeing exactly what would be involved.)
--
Ticket URL: <https://code.djangoproject.com/ticket/29969#comment:5>
Django
Reporter: Matija Kolarić | Owner: Carlton
| Gibson
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: 0 | UI/UX: 0
Comment (by Carlton Gibson):
Replying to [comment:5 Carlton Gibson]:
> Fix for #29930 ([https://github.com/django/django/pull/10678 PR]) would
"solve" this, in that the inline formset is removed when re-presenting the
parent form with the validation error.
>
> Question is whether that's good enough for this corner case? Or do we
want to represent the read-only inline? (2nd option is better; haven't yet
got to seeing exactly what would be involved.)
This is outdated. The PR there now maintains the inline when re-presenting
the form.
The issue here seems to be two part:
1. The form is re-presented bound to empty post data, submitted (or rather
not submitted) from the dummy form, rather than the initial instance data.
(First question is how to adjust that?)
2. When using save-as the form is presented as a form, rather than the
read-only alternative. (Not sure yet why that is.)
--
Ticket URL: <https://code.djangoproject.com/ticket/29969#comment:6>
Django
Reporter: Matija Kolarić | Owner: (none)
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* owner: Carlton Gibson => (none)
* status: assigned => new
Comment:
I'm going to de-assign myself here: I'll keep working on it but I'm not at
all sure I have a take on how best to handle it, so input welcome!
The difficulty is that:
1. For read-only fields we don't submit the post data needed to re-
populate the form (which for normal fields we do) and...
2. For `_saveasnew`,
[https://github.com/django/django/blob/f091ea35150d95fc6732bbf0c27b971dd445509a/django/contrib/admin/options.py#L1532-L1533
we don't have the parent object available] to fetch them again.
As such I don't see an easy/obvious fix.
--
Ticket URL: <https://code.djangoproject.com/ticket/29969#comment:7>
Django
Reporter: Matija Kolarić | Owner: (none)
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: 0 | UI/UX: 0
Changes (by Tim Graham):
* severity: Release blocker => Normal
Comment:
Yea, the fix seems complicated enough that we might not consider this a
release blocker. If someone provides a patch while 2.1 is still in bug fix
mode, we could consider it. `ModelAdmin._changeform_view()` could copy
`object_id` to another variable before it's set to `None` on the line you
mentioned. Then the inline relations need to be copied to the new object
somehow. Maybe ` ModelAdmin._create_formsets()` could do it if `obj` is
the original object and then the new object is swapped out for it.
--
Ticket URL: <https://code.djangoproject.com/ticket/29969#comment:8>
Django
Reporter: Matija Kolarić | Owner: (none)
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
Changes (by SnippyCodes):
* has_patch: 0 => 1
Ticket URL: <https://code.djangoproject.com/ticket/29969#comment:9>