Opened 7 years ago

Closed 7 years ago

#8882 closed (fixed)

Inline model formsets break validation for unique and unique_together with foreign keys

Reported by: central@… Owned by: brosner
Component: Forms Version: 1.0
Severity: Keywords: model-validation
Cc: gabor@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

The admin site fails when trying to create two registers (not unique) on a model with unique or unique together fields. It doesn't check for the unique statement and database backend raises an IntegrityError exception like this:

IntegrityError at /admin/myapp/mymodel/add/
(1062, "Duplicate entry '2-1' for key 2")

I haven't checked if the problem exists in "handmade" forms, but in admin site it fails.

Attachments (1)

formset-unique.diff (2.6 KB) - added by Alex 7 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 7 years ago by anonymous

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from Unique and Unique together in admin inlines is not working to Unique and Unique together in admin inlines are not working

comment:2 Changed 7 years ago by mtredinnick

Can you please construct a small example model that demonstrates the problem you are talking about. That way there's no chance of confusion as to whether we're debugging the problem you are reporting or not.

comment:3 Changed 7 years ago by mrts

  • Component changed from Admin interface to Forms
  • Triage Stage changed from Unreviewed to Accepted

#8890 is a duplicate. I up this to accepted as I, kratorius from #django-dev IRC, the person behind #8890 and current reporter are hit by this.

Will upload a minimal sample shortly.

comment:4 Changed 7 years ago by mrts

Also, see #8890 for a sample.

comment:5 Changed 7 years ago by mrts

  • Summary changed from Unique and Unique together in admin inlines are not working to Inline model formsets break validation for unique_together with foreign keys

Given the following models.py:

from django.db import models
from django import forms
from django.contrib import admin

class Foo(models.Model):
	foo = models.CharField(max_length=100)

# test for unique
class Bar(models.Model):
	foo = models.ForeignKey(Foo)
	bar = models.CharField(max_length=100, unique=True)

# test for unique_together
class Baz(models.Model):
	foo = models.ForeignKey(Foo)
	baz = models.CharField(max_length=100)

	class Meta:
		unique_together = ("foo", "baz")

class BarInline(admin.StackedInline):
	model = Bar

class BazInline(admin.StackedInline):
	model = Baz

class FooAdmin(admin.ModelAdmin):
	inlines = (BarInline, BazInline)

admin.site.register(Foo, FooAdmin)

Test case 1 -- the database is empty:

  • adding Bar.bar = 'a' twice results in IntegrityError at /admin/uniq/foo/add/ -- column bar is not unique,
  • adding a single Bar.bar = 'a' and consequently adding Baz.baz = 'a' twice results in IntegrityError at /admin/uniq/foo/1/ -- columns foo_id, baz are not unique

Test case 2 -- there is already a Bar with Bar.bar = 'a':

  • adding Bar.bar = 'a' again is correctly validated and Bar with this Bar already exists. displayed,
  • adding a single Bar.bar = 'b' and consequently adding Baz.baz = 'a' twice results in IntegrityError at /admin/uniq/foo/1/ -- columns foo_id, baz are not unique

comment:6 Changed 7 years ago by anonymous

  • Summary changed from Inline model formsets break validation for unique_together with foreign keys to Inline model formsets break validation for unique and unique_together with foreign keys

comment:7 Changed 7 years ago by ramiro

Something the reported in #8890 commented in that ticket that might be of some help: "It appears that the inlineformset_factory helper removes the foreign key field from the produced forms, which prevents validate_unique from actually validating like it is supposed to."

comment:8 Changed 7 years ago by Revil

This is a short slice of code from django/forms/models.py

    def validate_unique(self):
        from django.db.models.fields import FieldDoesNotExist

        # Gather a list of checks to perform. Since this is a ModelForm, some
        # fields may have been excluded; we can't perform a unique check on a
        # form that is missing fields involved in that check.

The real problem is that the foreign key to the "parent" model in the 1-* relation is missing

comment:9 Changed 7 years ago by anonymous

  • Cc gabor@… added

comment:10 Changed 7 years ago by mrts

  • Keywords model-validation added

comment:11 Changed 7 years ago by brosner

  • Owner changed from nobody to brosner
  • Status changed from new to assigned

Changed 7 years ago by Alex

comment:12 Changed 7 years ago by brosner

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [9297]) Fixed #8882 -- When a foreign key is among the unique_together fields in an inline formset properly handle it.

comment:13 Changed 7 years ago by brosner

(In [9298]) [1.0.X] Fixed #8882 -- When a foreign key is among the unique_together fields in an inline formset properly handle it.

Backport of r9297 from trunk.

comment:14 Changed 7 years ago by mrts

  • Resolution fixed deleted
  • Status changed from closed to reopened

brosner, the fix is unfortunately insufficient. Alex's patch is also needed, but as that deals with unique checks only, unique_together checks need still to be added to it.

Try the testcase that I've reported above.

I'll examine the code hopefully tomorrow and write a patch.

comment:15 Changed 7 years ago by Fugazi

This revision causes bug: http://dpaste.com/88174/.
Bug appears when Inline have fieldsets. In the same time, can you add this: http://code.djangoproject.com/ticket/9025 ?
Thank you.

comment:16 Changed 7 years ago by brosner

  • Resolution set to fixed
  • Status changed from reopened to closed

I suppose the problem that Alex was patching was indeed described here. There were really *two* issues hidden in this ticket (including all the tickets marked as duplicates). I fixed the one where the foreign key is involved in unique_together didn't work before. Lets not make this more confusing that it needs to be. To isolate the issues lets open new tickets. It will be much easier to track. Alex Gaynor has opened #9493 to track the issue mrts brought up in the reopen. It is a valid issue.

Fugazi, like I said above, lets open a new ticket with *detailed* information about the problem please. #9025 has absolutely nothing to do with any of this.

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