[Django] #33335: Django admin error when adding new object, when using Django 4 functional unique constraints on model
Django
unique constraints on model
-----------------------------------------+------------------------
Reporter: Hervé Le Roy | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: 4.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
When creating a new object from Django Administration site, I'm getting an
error "Tag with this already exists.". (Tag is my model name)
I'm using on this model the new functional unique constraints introducted
in Django 4.0
(https://docs.djangoproject.com/en/dev/ref/models/constraints/#django.db.models.UniqueConstraint).
I suppose this is related to the contraints put on the model. However,
creating a Tag object with code - e.g. `Tag.objects.create(...)` - works
fine. It only fails in Django admin. I'm not sure if it's a bug or
misunderstanding/misconfiguration on my side?
This is my model:
{{{
class Tag(models.Model):
"""Basic tag model."""
# When using get_or_create, we need to pass tag name as a default
value, like this:
# Tag.objects.get_or_create(defaults={'name': tag}, name__iexact=tag,
user=request.user)
user = models.ForeignKey(settings.AUTH_USER_MODEL,
on_delete=models.CASCADE, verbose_name=_('user'))
name = models.CharField(max_length=50, db_index=False,
verbose_name=_('name'),
blank=False, # It's the default behavior, but
let's be explicit
validators=[RegexValidator(regex=r'[A-z0-9À-ž\s]+',
message='This field
accepts only letters, digits and space.')])
slug = models.SlugField(max_length=50, verbose_name=_('slug'))
class Meta:
ordering = ['name']
# Make sure tag name:
# - are unique per user
# - are case insensitive (prevent adding "test" if "Test" already
exists)
# - aren't empty
constraints = [models.UniqueConstraint(fields=['user', 'name'],
name='%(app_label)s_%(class)s_name_unique_per_user'),
models.UniqueConstraint(Lower('name'),
name='%(app_label)s_%(class)s_name_case_insensitive'),
models.CheckConstraint(check=models.Q(name__length__gt=0),
name='%(app_label)s_%(class)s_name_not_empty')]
(snip)
}}}
This is the admin configuration:
{{{
@admin.register(Tag)
class TagAdmin(admin.ModelAdmin):
list_display = ('pk', 'user', 'name', 'slug')
list_display_links = ['pk']
fields = ('user', 'name',)
list_filter = ('user__email',)
search_fields = ('name',)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33335>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
unique constraints on model
Reporter: Hervé Le Roy | Owner: nobody
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* status: new => closed
* resolution: => needsinfo
Comment:
See TicketClosingReasons/UseSupportChannels for ways to get help debugging
the issue. Feel free to reopen if you confirm Django is at fault.
--
Ticket URL: <https://code.djangoproject.com/ticket/33335#comment:1>
Django
unique constraints on model
Reporter: Hervé Le Roy | Owner: nobody
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz):
Note you should also be able to test outside the admin with `tag =
Tag(...); tag.full_clean()`.
--
Ticket URL: <https://code.djangoproject.com/ticket/33335#comment:2>
Django
unique constraints on model
Reporter: Hervé Le Roy | Owner: nobody
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Hervé Le Roy):
Thank you Claude for pointing this out. I was testing outside of admin
using `tag = Tag(...); tag.save()`, which works.
But following your example, when I run `tag.full_clean()`, it fails with
the following error:
{{{
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/local/lib/python3.9/site-packages/django/db/models/base.py",
line 1261, in full_clean
raise ValidationError(errors)
django.core.exceptions.ValidationError: {'__all__': ['Tag with this
already exists.']}
}}}
The fact that there is a missing word between 'this' and 'already' is
weird. I have narrowed down the constraint causing the issue (when I
remove it, it works fine).
It's this one:
{{{
models.UniqueConstraint(Lower('name'),
name='%(app_label)s_%(class)s_name_case_insensitive'),
}}}
The ability to use positional argument *expressions in UniqueConstraint is
a new feature from Django 4.0
https://docs.djangoproject.com/en/4.0/ref/models/constraints/#expressions
I'm not expert enough to understand why `full_clean()`fails. We'll see
when 4.0 is released if I'm the only one facing this issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/33335#comment:3>
Django
unique constraints on model
Reporter: Hervé Le Roy | Owner: nobody
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz):
* Attachment "33335-test.diff" added.
Test code
Django
unique constraints on model
Reporter: Hervé Le Roy | Owner: nobody
Component: contrib.admin | Version: 4.0
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 Claude Paroz):
* status: closed => new
* type: Uncategorized => Bug
* resolution: needsinfo =>
* stage: Unreviewed => Accepted
Comment:
I can at least confirm an issue with the error message. As the index has
no direct field name, the pattern `A <object> with this <field_name>
already exists` can not work. I attached my code used for testing. In this
case, the `unique_error_message` should probably produce another
formulation a bit more generic, like "%(model_name)s is not unique in the
database".
I'm not sure if this is release blocker or not.
Hervé, your report suggest an issue also with the unicity check itself, I
was not able to reproduce that.
--
Ticket URL: <https://code.djangoproject.com/ticket/33335#comment:4>
Django
constraints.
---------------------------------+------------------------------------
Reporter: Hervé Le Roy | Owner: nobody
Component: contrib.admin | Version: 4.0
Severity: Release blocker | 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):
* cc: Hannes Ljungberg (added)
* severity: Normal => Release blocker
Comment:
I was able to confirm the issue with detecting uniqueness. It's a bug in
the new feature added in 3aa545281e0c0f9fac93753e3769df9e0334dbaa.
--
Ticket URL: <https://code.djangoproject.com/ticket/33335#comment:5>
Django
constraints.
---------------------------------+------------------------------------
Component: contrib.admin | Version: 4.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak):
[https://github.com/django/django/blob/d3f4c2b95d2a13a5d9bc0e6413dfdbab21388822/django/db/models/base.py#L1063-L1119
Model._get_unique_checks()] and related methods don't handle unique
constraints with expressions, because `_get_unique_checks()` returns a
constraint without fields which is violated when there are any rows. I can
try to solve it on Monday, unless someone does on the weekend.
--
Ticket URL: <https://code.djangoproject.com/ticket/33335#comment:6>
Django
constraints.
Reporter: Hervé Le Roy | Owner: nobody
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* component: contrib.admin => Database layer (models, ORM)
--
Ticket URL: <https://code.djangoproject.com/ticket/33335#comment:7>
Django
constraints.
-------------------------------------+-------------------------------------
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Hannes Ljungberg):
PR: https://github.com/django/django/pull/15154
Figured we should just exclude UniqueConstraints containing expressions
for any logic revolving around
[https://github.com/django/django/blob/93ba389d81b51ab9b511bae0813958397e1297cd/django/db/models/options.py#L860-L872
Options.total_unique_constraints()], we already discard conditional
constraints here. I can't imagine any realistic way to run the unique
validation checks against expression constraints.
--
Ticket URL: <https://code.djangoproject.com/ticket/33335#comment:8>
Django
constraints.
-------------------------------------+-------------------------------------
| Ljungberg
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Hannes Ljungberg):
* owner: nobody => Hannes Ljungberg
* status: new => assigned
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/33335#comment:9>
Django
constraints.
-------------------------------------+-------------------------------------
Reporter: Hervé Le Roy | Owner: Hannes
| Ljungberg
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak):
Replying to [comment:8 Hannes Ljungberg]:
> PR: https://github.com/django/django/pull/15154
>
> Figured we should just exclude UniqueConstraints containing expressions
for any logic revolving around
[https://github.com/django/django/blob/93ba389d81b51ab9b511bae0813958397e1297cd/django/db/models/options.py#L860-L872
Options.total_unique_constraints()], we already discard conditional
constraints here. I can't imagine any realistic way to run the unique
validation checks against expression constraints.
IMO it's feasible to validate against functional constraints, but it would
be tricky and it's too late to include this in Django 4.0. `expressions`
could be passed and handled separately in `Model._perform_unique_checks()`
for example by adding custom annotations 🤔 However, let's ignore them for
now.
--
Ticket URL: <https://code.djangoproject.com/ticket/33335#comment:10>
Django
constraints.
-------------------------------------+-------------------------------------
Reporter: Hervé Le Roy | Owner: Hannes
| Ljungberg
Type: Bug | Status: assigned
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/33335#comment:11>
Django
constraints.
-------------------------------------+-------------------------------------
Reporter: Hervé Le Roy | Owner: Hannes
| Ljungberg
Component: Database layer | Version: 4.0
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"1eaf38fa87384fe26d1abf6e389d6df1600d4d8c" 1eaf38f]:
{{{
#!CommitTicketReference repository=""
revision="1eaf38fa87384fe26d1abf6e389d6df1600d4d8c"
Fixed #33335 -- Made model validation ignore functional unique
constraints.
Regression in 3aa545281e0c0f9fac93753e3769df9e0334dbaa.
Thanks Hervé Le Roy for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33335#comment:12>
Django
constraints.
-------------------------------------+-------------------------------------
Reporter: Hervé Le Roy | Owner: Hannes
| Ljungberg
Type: Bug | Status: closed
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"fed7f992ac8b2ccb5164761a609277513f10d963" fed7f992]:
{{{
#!CommitTicketReference repository=""
revision="fed7f992ac8b2ccb5164761a609277513f10d963"
[4.0.x] Fixed #33335 -- Made model validation ignore functional unique
constraints.
Regression in 3aa545281e0c0f9fac93753e3769df9e0334dbaa.
Thanks Hervé Le Roy for the report.
Backport of 1eaf38fa87384fe26d1abf6e389d6df1600d4d8c from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33335#comment:13>