Re: [Django] #2259: PK Change creates new object instead of update

36 views
Skip to first unread message

Django

unread,
Aug 13, 2011, 3:28:07 PM8/13/11
to django-...@googlegroups.com
#2259: PK Change creates new object instead of update
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner: nobody
Type: Bug | Status: reopened
Milestone: | Component: contrib.admin
Version: | Severity: Normal
Resolution: | Keywords:
Triage Stage: Design | Has patch: 1
decision needed | Needs tests: 0
Needs documentation: 1 | Easy pickings: 0
Patch needs improvement: 1 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
Changes (by akaariai):

* cc: anssi.kaariainen@… (added)
* has_patch: 0 => 1
* ui_ux: => 0
* easy: => 0


Comment:

I know this is unlikely to get accepted because of backwards compatibility
reasons, but here goes anyways.

The attached patch implements a version of save_base that automatically
does the following things when the PK has been changed since last loaded
from the DB:
- If there is already a row with the new pk and force_update is not set,
it errors out.
- If there is already a row with the new pk and force_update is set it
updates the row with the new pk and leaves the old pk alone.
- If there is no row with new or old pk, it inserts a new row.
- If pk has not been changed, everything works as always.
- The only way (without resorting to undocumented features) to get a pk
update is to use a model that comes from DB. save() counts as come from
DB.

The above tries to be intelligent and do the right thing. The current code
also tries to be intelligent and do the right thing, but IMHO it fails
when the PK has been changed. What it does is not what one would expect.
On the other hand the specification of what save() does is very clear. I
think the problem here is that we have the single method save() which
should always do the right thing. With changing PKs it is not entirely
clear what the right thing is.

Other possible ways to solve this ticket is to make a new method or to add
additional optional parameters to save. In this case I think .save()
without force_update or force_insert should raise an Error if the primary
key has changed.

The biggest backwards incompatibility is this (present also in Django test
suite):
{{{
Lets save some objects:
obj = T()
t.pk = 123
t.name = 'foob'
t.save()
t.pk = 456
t.name = 'faab'
t.save()
# Or better yet, do that in loop...
}}}

Old code would have saved two objects, the new code updates the first
saved. So as is this patch has no change to get in. On the other hand,
if/when composite primary keys are going to be included in Django, the
current save() is going to cause a lot of wondering...

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:21>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Aug 17, 2011, 12:10:32 PM8/17/11
to django-...@googlegroups.com
#2259: PK Change creates new object instead of update
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner: nobody
Type: Bug | Status: reopened
Milestone: | Component: contrib.admin
Version: | Severity: Normal
Resolution: | Keywords:
Triage Stage: Design | Has patch: 1
decision needed | Needs tests: 0
Needs documentation: 1 | Easy pickings: 0
Patch needs improvement: 1 |
UI/UX: 0 |
-------------------------------------+-------------------------------------

Comment (by akaariai):

I have been thinking about this a bit... And to me it seems entirely clear
that the current situation where changing primary key inserts a new record
is not acceptable if/when composite primary keys are included in Django.
Composite primary keys mean more use of natural primary keys, and they do
have the habit of changing.

Here is an example why the current situation must be changed. Consider a
person model with a pk of first_name, last_name and the following code:

{{{
p = Person.objects.get(first_name='John', last_name='Smith')
p.first_name = 'Jack'
p.save()
}}}

Who expects that to end up with John and Jack in the DB?

As I see it, there are two choices:
1. Throw an error when saving an object with PK changed. The error should
say that Django does not know what you want in this situation, use kwargs
to specify the wanted action.
2. Make Django intelligent enough to just do the right thing. My patch
above tries to do that but I am afraid it is not good enough.

In both cases new kwargs to save are needed to make it possible to
specify/override the action on save().

I favor choice 1. Choice 2 will mean complicating the logic of save_base.
Also, can Django guess the right action in every case?

My suggestion: deprecate current behavior of save() in 1.4.

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:22>

Django

unread,
Sep 10, 2011, 6:53:48 PM9/10/11
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
------------------------------------+-------------------------------
Reporter: ed@… | Owner: nobody
Type: Bug | Status: reopened
Milestone: | Component: contrib.admin
Version: | Severity: Normal
Resolution: | Keywords:
Triage Stage: Accepted | Has patch: 1
Needs documentation: 1 | Needs tests: 0
Patch needs improvement: 1 | Easy pickings: 0
UI/UX: 0 |
------------------------------------+-------------------------------
Changes (by carljm):

* stage: Design decision needed => Accepted


Comment:

In discussion with Alex at the sprint, the need for cascading updates of
foreign keys pushes this beyond the bounds of what's reasonable for Django
to try to support. Primary keys should not change.

We should, however, update the admin so that primary key fields are
included in readonly_fields by default, to avoid the confusing and
unexpected creation of new records.

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:23>

Django

unread,
Mar 5, 2013, 12:18:13 AM3/5/13
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------+------------------------------------

Reporter: ed@… | Owner: nobody
Type: Bug | Status: reopened
Component: contrib.admin | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by streeter):

* cc: django@… (added)


Comment:

I was considering tackling this ticket, and had a question about a
possible implementation. Is this as simple as checking for a zero length
readonly_fields property and then adding in the attribute name for the pk
in the ModelAdmin constructor? The problem with this implementation, as I
see it, is it does not let the user override this value by setting
readonly_fields to () because we wouldn't be able to tell the difference
between the default value and an override.

Another option would be to include 'pk' in the default tuple for
readonly_fields, and then translate 'pk' into the attribute name for the
primary key in the constructor. Is this what you guys had in mind? This
second way seems more robust, but I wanted to check with you guys first.

I did a quick first pass on my branch here
https://github.com/streeter/django/tree/ticket-2259 (commit
https://github.com/streeter/django/commit/a9c039). I plan to add tests,
but want to make sure I'm on the right path first.

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:24>

Django

unread,
Nov 3, 2013, 3:38:08 PM11/3/13
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------+------------------------------------
Reporter: ed@… | Owner: nobody
Type: Bug | Status: new

Component: contrib.admin | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by bouke):

* cc: bouke (added)


Comment:

After diving into the logic responsible for generating the form, I suspect
that ModelForm generates the disputed form field. So it appears that the
fix should be in ModelForm, which shouldn't generate an editable field for
primary keys in the first place. However, then one wouldn't be able to
create a new instance from the admin as the non-editable field is
mandatory.

Another possibility would be to alter ``ModelAdmin.get_readonly_fields``
to return the primary key field for existing instances. Somewhat like
below, although that makes the normally hidden primary key fields visible
as well.

The first would be the most consistent solution; primary key fields never
editable. It would be up to the developers to dynamically add the primary
key field back in if needed.

{{{
def get_readonly_fields(self, request, obj=None):
"""
Hook for specifying custom readonly fields.
"""
if obj is None:
return self.readonly_fields
else:
return self.readonly_fields + (self.opts.pk.attname,)
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:26>

Django

unread,
Nov 3, 2013, 4:32:20 PM11/3/13
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------+------------------------------------
Reporter: ed@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------

Comment (by akaariai):

I've been thinking about this same issue when reviewing the composite
fields patch.

I think the right thing to do is:
1. Make primary keys non-editable when editing objects.
2. Make primary keys editable when inserting objects.
3. There should be opt-in for always editable and never editable.

The reason for voting for edit on insert ability is that otherwise the
generated add form is unusable. The pk field needs a value, but you can't
insert anything into them. That isn't user friendly default.

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:27>

Django

unread,
Nov 5, 2013, 9:49:51 AM11/5/13
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------+------------------------------------
Reporter: ed@… | Owner: bouke
Type: Bug | Status: assigned
Component: contrib.admin | Version: master

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by bouke):

* status: new => assigned
* needs_docs: 1 => 0
* version: => master
* owner: nobody => bouke
* needs_better_patch: 1 => 0


Comment:

PR: https://code.djangoproject.com/ticket/2259

I have added `BaseModelAdmin.auto_pk_readonly_fiel`, which controls
whether primary keys should be read-only when editing options. This should
cover all scenarios:

1. -- default --
2. -- default --
3. Either set `auto_pk_readonly_field=False` or include the field name in
`readonly_fields`

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:28>

Django

unread,
Nov 6, 2013, 5:13:41 AM11/6/13
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------+------------------------------------
Reporter: ed@… | Owner:
Type: Bug | Status: new

Component: contrib.admin | Version: master
Severity: Normal | 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 bouke):

* owner: bouke =>
* cc: bouke (removed)
* has_patch: 1 => 0
* status: assigned => new


Comment:

The proposed solution works for ModelAdmin forms, however it doesn't work
with InlineModelAdmin as `get_readonly_fields` is passed the parent
object, not the object regarding the InlineModelAdmin.

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:29>

Django

unread,
Feb 19, 2016, 9:30:46 AM2/19/16
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------+-------------------------------------
Reporter: ed@… | Owner: AMaini503
Type: Bug | Status: assigned

Component: contrib.admin | Version: master
Severity: Normal | 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 AMaini503):

* owner: => AMaini503


* status: new => assigned


--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:30>

Django

unread,
Feb 19, 2016, 9:47:27 AM2/19/16
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------+------------------------------------
Reporter: ed@… | Owner:
Type: Bug | Status: new
Component: contrib.admin | Version: master
Severity: Normal | 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 AMaini503):

* owner: AMaini503 =>


* status: assigned => new


--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:31>

Django

unread,
Jan 15, 2018, 3:31:56 PM1/15/18
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner: Pathangi
| Jatinshravan
Type: Bug | Status: assigned

Component: contrib.admin | Version: master
Severity: Normal | 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 Pathangi Jatinshravan):

* owner: (none) => Pathangi Jatinshravan


* status: new => assigned


--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:30>

Django

unread,
Jul 4, 2019, 5:26:09 AM7/4/19
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner: Pathangi
| Jatinshravan
Type: Bug | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: invalid

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 Chason Chaffin):

* status: assigned => closed
* resolution: => invalid


Comment:

Does not seem to be an issue in modern versions of Django. Attempts to add
the `primary_key` field to an Admin page will result in an error.

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:31>

Django

unread,
Jul 4, 2019, 5:44:53 AM7/4/19
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner: Pathangi
| Jatinshravan
Type: Bug | Status: new

Component: contrib.admin | Version: master
Severity: Normal | 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 felixxm):

* status: closed => new
* resolution: invalid =>


Comment:

This is still a valid issue.

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:32>

Django

unread,
Sep 14, 2019, 4:53:24 PM9/14/19
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner: Pathangi
| Jatinshravan
Type: Bug | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by michjnich):

Doesn't appear to be the same issue as it was to start with, if it still
exists at all. I replicated the original test in comment #1 using the
Group table, and it works fine now:

{{{
>>> from django.contrib.auth.models import Group
>>> Group.objects.all()
<QuerySet []>
>>> g = Group(name="test")
>>> g.save()
>>> Group.objects.all()
<QuerySet [<Group: test>]>
>>> g = Group.objects.get(name="test")
>>> g.name="testx"
>>> g.save()
>>> Group.objects.all()
<QuerySet [<Group: testx>]>
}}}

I then changed this back to "test" via the admin interface, which also
resulted in a single record called "test" without any issues. As far as I
can tell, the original issue no longer exists and this should be closed.

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:33>

Django

unread,
Sep 16, 2019, 4:46:18 AM9/16/19
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner: Pathangi
| Jatinshravan
Type: Bug | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by felixxm):

This issue is still valid please check
[https://code.djangoproject.com/ticket/2259#comment:27 Anssi's
proposition].

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:34>

Django

unread,
Dec 5, 2019, 8:58:07 AM12/5/19
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner: Pathangi
| Jatinshravan
Type: Bug | Status: new
Component: contrib.admin | Version: master
Severity: Normal | 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 jedie):

* cc: jedie (added)


Comment:

Note: https://code.djangoproject.com/ticket/14071 ''Row duplicated when
modifying PK'' is related: I add comment
https://code.djangoproject.com/ticket/14071#comment:2 to this about the
problem to change the primary key in admin.

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:35>

Django

unread,
May 10, 2021, 1:35:38 AM5/10/21
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------+------------------------------------
Reporter: ed@… | Owner: (none)
Type: Bug | Status: new
Component: contrib.admin | Version: dev

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------

Comment (by Mariusz Felisiak):

#32728 was a duplicate. Primary keys should not be allowed in the
`ModelAdmin.list_editable`.

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:36>

Django

unread,
Dec 11, 2021, 9:21:09 AM12/11/21
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner:
| siddhartha-star-dev
Type: Bug | Status: assigned

Component: contrib.admin | Version: dev
Severity: Normal | 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 siddhartha-star-dev):

* owner: (none) => siddhartha-star-dev


* status: new => assigned


--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:37>

Django

unread,
Dec 11, 2021, 11:31:36 AM12/11/21
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner:
| siddhartha-star-dev
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by siddhartha-star-dev):

Replying to [comment:34 Mariusz Felisiak]:


> This issue is still valid please check
[https://code.djangoproject.com/ticket/2259#comment:27 Anssi's
proposition].

Those comments primarily relate to admin right ? As far as I can see right
now, the following code snipped results in the same problem:
`obj = TestModel.objects.get(email="jon...@gmail.com")`
`obj.email = "j...@gmail.com"`
`obj.save()`
`exit()`

This creates a new object rather than updating the old one, should this
not be fixed right here ? That would mean that if and when we chose to
make the fields editable, we will just have to change how the form behaves
in the admin...

So should we just try and fix the case with `model.save()` first and then
move towards admin specifics ?

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:38>

Django

unread,
Dec 11, 2021, 2:17:26 PM12/11/21
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner:
| siddhartha-star-dev
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak):

> `obj.save()`
> `exit()`
> This creates a new object rather than updating the old one, should this
not be fixed right here?

This is a [https://docs.djangoproject.com/en/stable/ref/models/instances
/#how-django-knows-to-update-vs-insert documented] and expected behavior,
which shouldn't be changed.

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:39>

Django

unread,
Dec 15, 2021, 8:43:10 PM12/15/21
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner:
| siddhartha-star-dev
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by siddhartha-star-dev):

I come up with two ways to change primary key
1. we store old pk in hidden field in the change form and we check for the
hidden field's value and the current pk value, if they have changed, we
create a new object with the new pk and delete the old one.
2. there will be a popup which will open a new form where two fields will
be old pk and new pk, specifically to change the primary key if needed,
the idea is to have an "edit" option besides the pk field in the current
form which will then open a popup to change the pk of the given object

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:40>

Django

unread,
Dec 19, 2021, 11:52:46 PM12/19/21
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner:
| siddhartha-star-dev
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by siddhartha-star-dev):

Replying to [comment:39 Mariusz Felisiak]:
Please look at my [comment:40 comment] and reply if possible

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:41>

Django

unread,
Dec 20, 2021, 6:04:49 AM12/20/21
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner:
| siddhartha-star-dev
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak):

Replying to [comment:41 siddhartha-star-dev]:


> Replying to [comment:39 Mariusz Felisiak]:
> Please look at my [comment:40 comment] and reply if possible

Have you seen [https://code.djangoproject.com/ticket/2259#comment:27
Anssi's proposition]? We don't want to allow for editing primary keys in
admin. They should not be allowed in the `ModelAdmin.list_editable` and
become read-only when editing.

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:42>

Django

unread,
Dec 22, 2021, 10:31:49 AM12/22/21
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner:
| siddhartha-star-dev
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by siddhartha-star-dev):

Replying to [comment:42 Mariusz Felisiak]:


> Have you seen [https://code.djangoproject.com/ticket/2259#comment:27
Anssi's proposition]? We don't want to allow for editing primary keys in
admin. They should not be allowed in the `ModelAdmin.list_editable` and
become read-only when editing.

I have seen [https://code.djangoproject.com/ticket/2259#comment:27 Anssi's
proposition] , and I only put out those ways in reference to the 3 point
in the proposition (That editable should be an opt-in), if we are ok in
always having them non-editable, we should be good, I will start working
on that for now, we can visit the 3 point later....

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:43>

Django

unread,
Feb 19, 2022, 12:20:45 PM2/19/22
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner:
| siddhartha-star-dev
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* has_patch: 0 => 1

* needs_tests: 0 => 1


Comment:

[https://github.com/django/django/pull/15440 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:44>

Django

unread,
Apr 25, 2022, 5:05:23 AM4/25/22
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner:
| siddhartha-star-dev
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* needs_tests: 1 => 0
* stage: Accepted => Ready for checkin


--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:45>

Django

unread,
Apr 25, 2022, 7:40:55 AM4/25/22
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner:
| siddhartha-star-dev
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"dcebc5da4831d2982b26d00a9480ad538b5c5acf" dcebc5da]:
{{{
#!CommitTicketReference repository=""
revision="dcebc5da4831d2982b26d00a9480ad538b5c5acf"
Refs #2259 -- Disallowed primary keys in ModelAdmin.list_editable.

Refs #32728.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:46>

Django

unread,
Apr 25, 2022, 7:41:16 AM4/25/22
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner:
| siddhartha-star-dev
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | 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 Mariusz Felisiak):

* has_patch: 1 => 0
* stage: Ready for checkin => Accepted


--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:47>

Django

unread,
Aug 1, 2023, 8:01:29 AM8/1/23
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------+------------------------------------
Reporter: ed@… | Owner: (none)
Type: Bug | Status: new
Component: contrib.admin | Version: dev
Severity: Normal | 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 Mariusz Felisiak):

* owner: siddhartha-star-dev => (none)


* status: assigned => new


--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:48>

Django

unread,
Nov 21, 2023, 12:45:06 PM11/21/23
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------+-------------------------------------------
Reporter: ed@… | Owner: Prashant Pandey
Type: Bug | Status: assigned

Component: contrib.admin | Version: dev
Severity: Normal | 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 Prashant Pandey):

* owner: (none) => Prashant Pandey


* status: new => assigned


--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:49>

Django

unread,
Nov 22, 2023, 12:31:43 PM11/22/23
to django-...@googlegroups.com
#2259: Primary keys should be readonly by default in admin
-------------------------------+-------------------------------------------
Reporter: ed@… | Owner: Prashant Pandey
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-------------------------------------------

Comment (by Prashant Pandey):

Replying to [comment:28 Bouke Haarsmay]:
Hi, I was following your solution and it gives same errors that you are
getting(regression error for inline).
I just need help in that area.
Is there a way to access the context of the form inside the
get_readonly_fields, means when we do:
1. Create new object where primary key field will be in editing state.
2. If we hit the change behaviour it should be readonly.

we can add a condition in the get_readonly_fields function where it will
send pk as a readonly if user wants to modify the object.

--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:50>

Reply all
Reply to author
Forward
0 new messages