Opened 7 years ago

Last modified 3 months ago

#28589 assigned Cleanup/optimization

When saving in admin, readonly fields are potentially overwritten

Reported by: Fingal Plumpton Owned by: Fingal Plumpton
Component: contrib.admin Version: dev
Severity: Normal Keywords: Admin SQL Overwrite
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description (last modified by Fingal Plumpton)

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.

Change History (7)

comment:1 by Fingal Plumpton, 7 years ago

Owner: changed from nobody to Fingal Plumpton
Status: newassigned

comment:2 by Fingal Plumpton, 7 years ago

Summary: When saving in admin, readonly fields are saved againWhen saving in admin, readonly fields are potentially overwritten

comment:3 by Fingal Plumpton, 7 years ago

Description: modified (diff)

comment:4 by Fingal Plumpton, 7 years ago

Description: modified (diff)

comment:5 by Fingal Plumpton, 7 years ago

My patch is available at branch ticket_28589 of my fork of Django https://github.com/FingalP/django

Version 0, edited 7 years ago by Fingal Plumpton (next)

comment:6 by Tim Graham, 7 years ago

Easy pickings: unset
Patch needs improvement: set
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

Recently, someone raised 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.

comment:7 by Alexander Lazarević, 3 months ago

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)
...
Note: See TracTickets for help on using tickets.
Back to Top