Opened 10 days ago

Last modified 5 days ago

#35551 closed Bug

Fix changing PK via admin — at Version 10

Reported by: Csirmaz Bendegúz Owned by: Csirmaz Bendegúz
Component: contrib.admin Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Csirmaz Bendegúz)

I have noticed two bugs when changing the primary key field via admin.

Given:

# models.py
class Dummy(models.Model):
    id = models.IntegerField(primary_key=True)

# admin.py
admin.site.register(Dummy)
  1. Create Dummy (1), Dummy (2).
  2. Change Dummy (2)'s Id field -> 1
  3. Dummy (1) is overwritten with Dummy (2), Dummy(2) stays unchanged.
  4. Change Dummy (1)'s Id field -> 3
  5. A new Dummy (3) is created, Dummy (1) stays unchanged.

I believe both step 3 and step 5 are bugs.

Change History (10)

comment:1 by Csirmaz Bendegúz, 10 days ago

Description: modified (diff)

comment:2 by Sarah Boyce, 8 days ago

Triage Stage: Unreviewed → Accepted

Great catch thank you!

I believe step 5 should never happen, since step 3 should fail with a unique check error.

Agreed 👍 if you were to add another field to the model then you slightly override Dummy (1) with whatever is in Dummy (2) - which is not nice.

comment:3 by Csirmaz Bendegúz, 8 days ago

Owner: changed from nobody to Csirmaz Bendegúz
Status: new → assigned

comment:4 by Csirmaz Bendegúz, 8 days ago

So actually there are 2 issues:

  1. The unique check is skipped when PK is changed.
  2. The object is duplicated when PK is changed.

I'm looking into this.

comment:5 by Csirmaz Bendegúz, 8 days ago

Description: modified (diff)

comment:6 by Csirmaz Bendegúz, 8 days ago

Description: modified (diff)

comment:7 by Csirmaz Bendegúz, 7 days ago

Has patch: set

comment:8 by Csirmaz Bendegúz, 7 days ago

I have submitted a basic fix for raising a ValidationError on PK conflict.

The other issue I would like to address is the fact that Django creates a duplicate if there's no PK conflict.

Should I create another ticket for this?

Last edited 7 days ago by Csirmaz Bendegúz (previous) (diff)

comment:9 by Csirmaz Bendegúz, 7 days ago

I submitted another PR for the above, please review both! Thanks!

Last edited 6 days ago by Csirmaz Bendegúz (previous) (diff)

comment:10 by Csirmaz Bendegúz, 6 days ago

Description: modified (diff)
Summary: Unique checks skipped when changing primary key → Fix changing PK via admin

I edited the ticket to reflect my findings.

Note: See TracTickets for help on using tickets.
Back to Top