[Django] #28589: When saving in admin, readonly fields are saved again
Django
-------------------------------------+-------------------------------------
Reporter: FingalP | Owner: nobody
Type: Bug | Status: new
Component: | Version: master
contrib.admin |
Severity: Normal | Keywords: Admin SQL Overwrite
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
On the admin page for a model, if a field is in readonly_fields, then
saving that model from the admin will create an SQL query that sets the
readonly field to it's value at the time the save button was clicked. This
field might have been changed (e.g. by a command or some other process)
during the time that the model is being saved, and with current behaviour
it would then be overwritten to it's old value.
Correct behaviour would be to not set the readonly field at all in the SQL
query.
--
Ticket URL: <https://code.djangoproject.com/ticket/28589>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: Admin SQL Overwrite | Triage Stage:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => FingalP
--
Ticket URL: <https://code.djangoproject.com/ticket/28589#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: FingalP | Owner: FingalP
Type: Bug | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
--
Ticket URL: <https://code.djangoproject.com/ticket/28589#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: FingalP | Owner: FingalP
Type: Bug | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> On the admin page for a model, if a field is in readonly_fields, then
> saving that model from the admin will create an SQL query that sets the
> readonly field to it's value at the time the save button was clicked.
> This field might have been changed (e.g. by a command or some other
> process) during the time that the model is being saved, and with current
> behaviour it would then be overwritten to it's old value.
>
> Correct behaviour would be to not set the readonly field at all in the
> SQL query.
New description:
On the admin page for a model, if a field is in readonly_fields or has
editable=False, then saving that model from the admin will create an SQL
query that sets the readonly field to it's value at the time the save
button was clicked. This field might have been changed (e.g. by a command
or some other process) during the time that the model is being saved, and
with current behaviour it would then be overwritten to it's old value.
Correct behaviour would be to not set the readonly field at all in the SQL
query.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28589#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: FingalP | Owner: FingalP
Type: Bug | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> On the admin page for a model, if a field is in readonly_fields or has
> editable=False, then saving that model from the admin will create an SQL
> query that sets the readonly field to it's value at the time the save
> button was clicked. This field might have been changed (e.g. by a command
> or some other process) during the time that the model is being saved, and
> with current behaviour it would then be overwritten to it's old value.
>
> Correct behaviour would be to not set the readonly field at all in the
> SQL query.
New description:
On the admin page for a model, if a field is in readonly_fields or has
editable=False, then saving that model from the admin will create an SQL
query that sets the readonly field to it's value at the time the save
button was clicked. This field might have been changed (e.g. by a command
or some other process) during the time that the model is being saved, and
with current behaviour it would then be overwritten to it's old value.
Correct behaviour would be to not set the readonly/uneditable field at all
in the SQL query.
The exception to this is a DateTimeField with auto_now or auto_now_add,
which have editable=False but should be updated when the SQL query is
made.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28589#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: FingalP | Owner: FingalP
Type: Bug | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by FingalP):
My patch is available at branch ticket_28589 of my fork of Django
https://github.com/FingalP/django
--
Ticket URL: <https://code.djangoproject.com/ticket/28589#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Fingal Plumpton | Owner: Fingal
Type: | Plumpton
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
* type: Bug => Cleanup/optimization
* easy: 1 => 0
* stage: Unreviewed => Accepted
Comment:
Recently, someone raised [https://groups.google.com/d/topic/django-
users/oYYqT7SIJVg/discussion a similar issue for model forms]. I think the
problem should be solved at that level first. Perhaps that'll help
simplify the code at the admin level -- the current code looks too
complicated.
--
Ticket URL: <https://code.djangoproject.com/ticket/28589#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Fingal Plumpton | Owner: Fingal
Type: | Plumpton
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: Admin SQL Overwrite | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Alexander Lazarević):
So for what it's worth, this seems to be still the case in
5.1.dev20240112204018:
{{{
UPDATE "empty_somemodel"
SET "name" = '''wowzer'',
"description" = '''''',
"choices" = '''''',
"random_tag" = '''9436''',
"date" = '''2024-01-01''',
"time" = '''11:18:48''',
"boolean" = 'False',
"decimal" = '''0.01''',
"floating" = '1.0',
"json" = '''{}'''
WHERE "empty_somemodel"."id" = '1'
}}}
{{{
class SomeModel(models.Model):
...
random_tag = models.CharField(null=False, max_length=32,
default=random_text, editable=False)
...
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28589#comment:7>