Fixed #2259 -- Made manually specified primary keys readonly in the admin. #19675
Conversation
…omatic zoom issue in mobile Safari.
…dmin change forms.
Jkhall81
force-pushed
the
2259-readonly-manual-pk-admin
branch
from
094817a to
7a78c93
Compare
| @@ -418,7 +418,22 @@ def get_readonly_fields(self, request, obj=None): | |||
| """ | |||
| Hook for specifying custom readonly fields. | |||
| """ | |||
| return self.readonly_fields | |||
There was a problem hiding this comment.
It works on my test project, and it should not affect existing overrides. It also provides the option to fall back to the initial behavior:
def get_readonly_fields(self, request, obj=None):
return self.readonly_fields
django/contrib/admin/options.py
Outdated
|
|
||
| # Only add PK to readonly if: | ||
| # - editing an existing object | ||
| # - PK is not auto-created |
There was a problem hiding this comment.
Any particular reason why an auto_created pk needs to be excluded?
There was a problem hiding this comment.
auto_created pks are already not editable in the admin.
There was a problem hiding this comment.
Thank you for the PR
This doesn't seem to be working fully for inlines.
Given the models:
class State(models.Model):
label = models.CharField(max_length=255)
class StatePKModel(models.Model):
id = models.IntegerField(primary_key=True)
state = models.ForeignKey(State, related_name="pk_state_model", on_delete=models.CASCADE)and admin
class StatePKModelInline(admin.TabularInline):
model = StatePKModel
@admin.register(State)
class StateAdmin(admin.ModelAdmin):
inlines = [StatePKModelInline,]I can edit the id in the inline
| label { | ||
| font-size: 1rem; | ||
| } | ||
|
|
There was a problem hiding this comment.
This change looks to be unrelated to the ticket
There was a problem hiding this comment.
That was my bad. Fixed this css thing (was from a year old PR). Also, I confirmed with a test that manually defined Pks are readonly in inlines when editing existing objects.
django/contrib/admin/options.py
Outdated
| @@ -418,7 +418,22 @@ def get_readonly_fields(self, request, obj=None): | |||
| """ | |||
| Hook for specifying custom readonly fields. | |||
| """ | |||
| return self.readonly_fields | |||
| readonly = self.readonly_fields | |||
| pk_field = self.model._meta.pk | |||
There was a problem hiding this comment.
This should account for composite primary keys now and use _meta.pk_fields.
There was a problem hiding this comment.
Thanks! Updated to use _meta.pk_fields for future composite PK support, with fallback to _meta.pk for compatibility.
There was a problem hiding this comment.
Updated to use _meta.pk_fields for future composite PK support, with fallback to _meta.pk for compatibility.
No need for a fallback, see the docs.
Jkhall81
force-pushed
the
2259-readonly-manual-pk-admin
branch
from
893d265 to
262ca22
Compare
Jkhall81
force-pushed
the
2259-readonly-manual-pk-admin
branch
from
262ca22 to
2153973
Compare
Trac ticket number
https://code.djangoproject.com/ticket/2259
Branch description
When editing a model instance in the Django admin, manually specified primary key fields (e.g.
CharField(primary_key=True)) should not be editable. Without this fix, users can mistakenly change a model's PK, which can cause errors or lead to duplicate records.This patch updates
BaseModelAdmin.get_readonly_fields()to automatically mark such PKs as readonly if they are not auto-generated and not already in the readonly fields.This change was verified against existing admin inline tests (test_char_pk_inline, test_integer_pk_inline), which initially failed due to the patch, and the logic was adjusted to restore their passing state.
Checklist
mainbranch.