Re: UniqueConstraint validation error message conditional vs non-conditional

2022-09-13 Thread charettes
Hello David,

The proposed patch and test adjustments make sense to me, please open an 
associated ticket and PR.

Thanks for digging this through and working on a solution!

I think that an argument could be made for the `if e.code == "unique" and 
len(constraint.fields) == 1` branch in `Model.validate_constraints` to be 
entirely encapsulated in `UniqueConstraint.validate` (which knows about 
len(self.fields)) but focusing on making sure the proper code is attributed 
to the ValidationError instance is definitely an improvement.

Cheers,
Simon

Le mardi 13 septembre 2022 à 09:40:59 UTC-4, amolras...@gmail.com a écrit :

> Then what I do..? 
>
> On Mon, 12 Sep, 2022, 8:36 pm David Sanders,  
> wrote:
>
>> Hi Simon,
>>
>> > This should already work for constraints over a single field but not on 
>> the ones with multiple fields[0] which is covered by the suite[1] but it 
>> doesn't look like UniqueConstraint.validate is providing code="unique" 
>> which might be the source of the issue you are encountering?
>>
>> Yep that's correct. The code="unique" is only ever set from 
>> Model.unique_error_message() 
>> 
>>  
>> which, as you're already aware, is only called if there's no condition in 
>> UniqueConstraint.validate() 
>> .
>>  
>> Updating one of the tests confirms this (see below).
>>
>> Given that we'd like to avoid wording complications with conditions… 
>> looks like we could simply add code="unique" in UniqueConstraint.validate() 
>> as per:
>>
>> index 49c7c91de9..61ca15c7c4 100644
>> --- a/django/db/models/constraints.py
>> +++ b/django/db/models/constraints.py
>> @@ -364,6 +364,8 @@ class UniqueConstraint(BaseConstraint):
>>  if (self.condition & 
>> Exists(queryset.filter(self.condition))).check(
>>  against, using=using
>>  ):
>> -raise 
>> ValidationError(self.get_violation_error_message())
>> +raise ValidationError(
>> +message=self.get_violation_error_message(), 
>> code="unique"
>> +)
>>  except FieldError:
>>  pass
>>
>> Which then fixes the patched test:
>>
>> index d4054dfd77..d433555f9f 100644
>> --- a/tests/constraints/tests.py
>> +++ b/tests/constraints/tests.py
>> @@ -569,8 +569,9 @@ class UniqueConstraintTests(TestCase):
>>  name=obj1.name, color="blue"
>>  ).validate_constraints()
>>  msg = "Constraint “name_without_color_uniq” is violated."
>> -with self.assertRaisesMessage(ValidationError, msg):
>> +with self.assertRaisesMessage(ValidationError, msg) as cm:
>>  UniqueConstraintConditionProduct(name=obj2.name
>> ).validate_constraints()
>> +self.assertEqual(cm.exception.message_dict, {"name": [msg]})
>>
>>  def test_validate(self):
>>  constraint = UniqueConstraintProduct._meta.constraints[0]
>>
>> (pasting test failure)
>>
>> ==
>> FAIL: test_model_validation_with_condition 
>> (constraints.tests.UniqueConstraintTests)
>> Partial unique constraints are not ignored by
>> --
>> Traceback (most recent call last):
>> ...
>> AssertionError: {'__all__': ['Constraint “name_without_color_uniq” is 
>> violated.']} != {'name': ['Constraint “name_without_color_uniq” is 
>> violated.']}
>> - {'__all__': ['Constraint “name_without_color_uniq” is violated.']}
>> ?   ^^ 
>>
>> + {'name': ['Constraint “name_without_color_uniq” is violated.']}
>> ?   ^ ^^
>>
>>
>> If this is acceptable I could open a ticket / start a PR?
>>
>> --
>> David
>>
>> On Mon, 12 Sept 2022 at 23:26, charettes  wrote:
>>
>>> Hello David
>>>
>>> > Would it be possible to group the message by field in the same way as 
>>> standard unique?
>>>
>>> This should already work for constraints over a single field but not on 
>>> the ones with multiple fields[0] which is covered by the suite[1] but it 
>>> doesn't look like UniqueConstraint.validate is providing code="unique" 
>>> which might be the source of the issue you are encountering?
>>>
>>> [0] 
>>> https://github.com/django/django/blob/07ebef566f751e172e266165071081c7614e2d33/django/db/models/base.py#L1443-L1447
>>> [1] https://djangoci.com/job/django-coverage/HTML_20Coverage_20Report/
>>>
>>>
>>> Le dimanche 11 septembre 2022 à 22:02:35 UTC-4, shang.xia...@gmail.com 
>>> a écrit :
>>>
 Hi Simon,

 Cheers for the explanation.

 I'm ok with the error message being the "constraint is violated" 
 generic message as I agree with what you're saying.

 Would it be possible to group the message by field in the same way as 
 standard unique?
>

Re: UniqueConstraint validation error message conditional vs non-conditional

2022-09-13 Thread Amol Rashinkar
Then what I do..?

On Mon, 12 Sep, 2022, 8:36 pm David Sanders, 
wrote:

> Hi Simon,
>
> > This should already work for constraints over a single field but not on
> the ones with multiple fields[0] which is covered by the suite[1] but it
> doesn't look like UniqueConstraint.validate is providing code="unique"
> which might be the source of the issue you are encountering?
>
> Yep that's correct. The code="unique" is only ever set from
> Model.unique_error_message()
> 
> which, as you're already aware, is only called if there's no condition in
> UniqueConstraint.validate()
> .
> Updating one of the tests confirms this (see below).
>
> Given that we'd like to avoid wording complications with conditions… looks
> like we could simply add code="unique" in UniqueConstraint.validate() as
> per:
>
> index 49c7c91de9..61ca15c7c4 100644
> --- a/django/db/models/constraints.py
> +++ b/django/db/models/constraints.py
> @@ -364,6 +364,8 @@ class UniqueConstraint(BaseConstraint):
>  if (self.condition &
> Exists(queryset.filter(self.condition))).check(
>  against, using=using
>  ):
> -raise
> ValidationError(self.get_violation_error_message())
> +raise ValidationError(
> +message=self.get_violation_error_message(),
> code="unique"
> +)
>  except FieldError:
>  pass
>
> Which then fixes the patched test:
>
> index d4054dfd77..d433555f9f 100644
> --- a/tests/constraints/tests.py
> +++ b/tests/constraints/tests.py
> @@ -569,8 +569,9 @@ class UniqueConstraintTests(TestCase):
>  name=obj1.name, color="blue"
>  ).validate_constraints()
>  msg = "Constraint “name_without_color_uniq” is violated."
> -with self.assertRaisesMessage(ValidationError, msg):
> +with self.assertRaisesMessage(ValidationError, msg) as cm:
>  UniqueConstraintConditionProduct(name=obj2.name
> ).validate_constraints()
> +self.assertEqual(cm.exception.message_dict, {"name": [msg]})
>
>  def test_validate(self):
>  constraint = UniqueConstraintProduct._meta.constraints[0]
>
> (pasting test failure)
>
> ==
> FAIL: test_model_validation_with_condition
> (constraints.tests.UniqueConstraintTests)
> Partial unique constraints are not ignored by
> --
> Traceback (most recent call last):
> ...
> AssertionError: {'__all__': ['Constraint “name_without_color_uniq” is
> violated.']} != {'name': ['Constraint “name_without_color_uniq” is
> violated.']}
> - {'__all__': ['Constraint “name_without_color_uniq” is violated.']}
> ?   ^^ 
>
> + {'name': ['Constraint “name_without_color_uniq” is violated.']}
> ?   ^ ^^
>
>
> If this is acceptable I could open a ticket / start a PR?
>
> --
> David
>
> On Mon, 12 Sept 2022 at 23:26, charettes  wrote:
>
>> Hello David
>>
>> > Would it be possible to group the message by field in the same way as
>> standard unique?
>>
>> This should already work for constraints over a single field but not on
>> the ones with multiple fields[0] which is covered by the suite[1] but it
>> doesn't look like UniqueConstraint.validate is providing code="unique"
>> which might be the source of the issue you are encountering?
>>
>> [0]
>> https://github.com/django/django/blob/07ebef566f751e172e266165071081c7614e2d33/django/db/models/base.py#L1443-L1447
>> [1] https://djangoci.com/job/django-coverage/HTML_20Coverage_20Report/
>>
>>
>> Le dimanche 11 septembre 2022 à 22:02:35 UTC-4, shang.xia...@gmail.com a
>> écrit :
>>
>>> Hi Simon,
>>>
>>> Cheers for the explanation.
>>>
>>> I'm ok with the error message being the "constraint is violated" generic
>>> message as I agree with what you're saying.
>>>
>>> Would it be possible to group the message by field in the same way as
>>> standard unique?
>>>
>>> ie, would this be an idea?:
>>>
>>> django.core.exceptions.ValidationError: {'test': ['Constraint “unique”
>>> is violated.']}
>>>
>>> This way the customised error message would be preserved.
>>>
>>> This also leads to another question I had about whether it might be an
>>> idea to allow overriding error messages for constraints in forms using the
>>> error_messages dict… but I'll leave that for another post :)
>>>
>>> Regards,
>>> David
>>>
>>> On Mon, 12 Sept 2022 at 11:53, charettes  wrote:
>>>
 Hello David,

 This is expected because Django doesn't have a way to express the
 constraint in words to present to the user when a condition, which could be
 complex, is provided.

 When no conditions are defined the metadata is easy to in

Re: UniqueConstraint validation error message conditional vs non-conditional

2022-09-12 Thread David Sanders
Hi Simon,

> This should already work for constraints over a single field but not on
the ones with multiple fields[0] which is covered by the suite[1] but it
doesn't look like UniqueConstraint.validate is providing code="unique"
which might be the source of the issue you are encountering?

Yep that's correct. The code="unique" is only ever set from
Model.unique_error_message()

which, as you're already aware, is only called if there's no condition in
UniqueConstraint.validate()
.
Updating one of the tests confirms this (see below).

Given that we'd like to avoid wording complications with conditions… looks
like we could simply add code="unique" in UniqueConstraint.validate() as
per:

index 49c7c91de9..61ca15c7c4 100644
--- a/django/db/models/constraints.py
+++ b/django/db/models/constraints.py
@@ -364,6 +364,8 @@ class UniqueConstraint(BaseConstraint):
 if (self.condition &
Exists(queryset.filter(self.condition))).check(
 against, using=using
 ):
-raise
ValidationError(self.get_violation_error_message())
+raise ValidationError(
+message=self.get_violation_error_message(),
code="unique"
+)
 except FieldError:
 pass

Which then fixes the patched test:

index d4054dfd77..d433555f9f 100644
--- a/tests/constraints/tests.py
+++ b/tests/constraints/tests.py
@@ -569,8 +569,9 @@ class UniqueConstraintTests(TestCase):
 name=obj1.name, color="blue"
 ).validate_constraints()
 msg = "Constraint “name_without_color_uniq” is violated."
-with self.assertRaisesMessage(ValidationError, msg):
+with self.assertRaisesMessage(ValidationError, msg) as cm:
 UniqueConstraintConditionProduct(name=obj2.name
).validate_constraints()
+self.assertEqual(cm.exception.message_dict, {"name": [msg]})

 def test_validate(self):
 constraint = UniqueConstraintProduct._meta.constraints[0]

(pasting test failure)

==
FAIL: test_model_validation_with_condition
(constraints.tests.UniqueConstraintTests)
Partial unique constraints are not ignored by
--
Traceback (most recent call last):
...
AssertionError: {'__all__': ['Constraint “name_without_color_uniq” is
violated.']} != {'name': ['Constraint “name_without_color_uniq” is
violated.']}
- {'__all__': ['Constraint “name_without_color_uniq” is violated.']}
?   ^^ 

+ {'name': ['Constraint “name_without_color_uniq” is violated.']}
?   ^ ^^


If this is acceptable I could open a ticket / start a PR?

--
David

On Mon, 12 Sept 2022 at 23:26, charettes  wrote:

> Hello David
>
> > Would it be possible to group the message by field in the same way as
> standard unique?
>
> This should already work for constraints over a single field but not on
> the ones with multiple fields[0] which is covered by the suite[1] but it
> doesn't look like UniqueConstraint.validate is providing code="unique"
> which might be the source of the issue you are encountering?
>
> [0]
> https://github.com/django/django/blob/07ebef566f751e172e266165071081c7614e2d33/django/db/models/base.py#L1443-L1447
> [1] https://djangoci.com/job/django-coverage/HTML_20Coverage_20Report/
>
>
> Le dimanche 11 septembre 2022 à 22:02:35 UTC-4, shang.xia...@gmail.com a
> écrit :
>
>> Hi Simon,
>>
>> Cheers for the explanation.
>>
>> I'm ok with the error message being the "constraint is violated" generic
>> message as I agree with what you're saying.
>>
>> Would it be possible to group the message by field in the same way as
>> standard unique?
>>
>> ie, would this be an idea?:
>>
>> django.core.exceptions.ValidationError: {'test': ['Constraint “unique” is
>> violated.']}
>>
>> This way the customised error message would be preserved.
>>
>> This also leads to another question I had about whether it might be an
>> idea to allow overriding error messages for constraints in forms using the
>> error_messages dict… but I'll leave that for another post :)
>>
>> Regards,
>> David
>>
>> On Mon, 12 Sept 2022 at 11:53, charettes  wrote:
>>
>>> Hello David,
>>>
>>> This is expected because Django doesn't have a way to express the
>>> constraint in words to present to the user when a condition, which could be
>>> complex, is provided.
>>>
>>> When no conditions are defined the metadata is easy to interpret to form
>>> a sentence out of ("Foo with this field0, field1, fieldN already exists")
>>> while when a condition is provided Q can define very complex criteria that
>>> only the developer is able to express in words. This is the same class of
>>> problem as try

Re: UniqueConstraint validation error message conditional vs non-conditional

2022-09-12 Thread charettes
Hello David

> Would it be possible to group the message by field in the same way as 
standard unique?

This should already work for constraints over a single field but not on the 
ones with multiple fields[0] which is covered by the suite[1] but it 
doesn't look like UniqueConstraint.validate is providing code="unique" 
which might be the source of the issue you are encountering?

[0] 
https://github.com/django/django/blob/07ebef566f751e172e266165071081c7614e2d33/django/db/models/base.py#L1443-L1447
[1] https://djangoci.com/job/django-coverage/HTML_20Coverage_20Report/


Le dimanche 11 septembre 2022 à 22:02:35 UTC-4, shang.xia...@gmail.com a 
écrit :

> Hi Simon,
>
> Cheers for the explanation.
>
> I'm ok with the error message being the "constraint is violated" generic 
> message as I agree with what you're saying.
>
> Would it be possible to group the message by field in the same way as 
> standard unique?
>
> ie, would this be an idea?:
>
> django.core.exceptions.ValidationError: {'test': ['Constraint “unique” is 
> violated.']}
>
> This way the customised error message would be preserved.
>
> This also leads to another question I had about whether it might be an 
> idea to allow overriding error messages for constraints in forms using the 
> error_messages dict… but I'll leave that for another post :)
>
> Regards,
> David
>
> On Mon, 12 Sept 2022 at 11:53, charettes  wrote:
>
>> Hello David,
>>
>> This is expected because Django doesn't have a way to express the 
>> constraint in words to present to the user when a condition, which could be 
>> complex, is provided.
>>
>> When no conditions are defined the metadata is easy to interpret to form 
>> a sentence out of ("Foo with this field0, field1, fieldN already exists") 
>> while when a condition is provided Q can define very complex criteria that 
>> only the developer is able to express in words. This is the same class of 
>> problem as trying to have a machine describe a text representation of a 
>> regular expression. It's doable for very simple cases but almost impossible 
>> for complex ones.
>>
>> We decided not to try to be clever about what the error message should be 
>> and opted to have a default that uses the constraint name when a condition 
>> is present while allowing developers to provide a `violation_error_message` 
>> at constraint initialization to have them express what a more appropriate 
>> error message should be.
>>
>> If we take your particular example, how should Django figure out what the 
>> error message should be for a UniqueConstraint of this form?
>>
>> UniqueConstraint(
>> fields=["foo"],
>> name="unique_uppercase_foo",
>> condition=Q(test__upper=F("test"))
>> )
>>
>> The developer who defined the constraint is in a much better position to 
>> provide an error that says that "Test with an upper case "foo" must be 
>> unique". If you're not convinced try with a complex condition involving OR 
>> and mixing transform comparison (e.g. a unique constraint only defined on 
>> palindromes)
>>
>> Cheers,
>> Simon
>>
>> Le dimanche 11 septembre 2022 à 14:52:47 UTC-4, othnield...@gmail.com a 
>> écrit :
>>
>>> nice one there
>>>
>>> On Sun, Sep 11, 2022 at 3:36 PM David Sanders  
>>> wrote:
>>>
 Hi folks (and in particular Simon Charette),

 I had a bit of a gotcha moment when a custom unique constraint 
 validation message disappeared when I added a condition to it. I won't 
 raise a ticket for this because it looks intentional from the constraint 
 validation PR, but I wanted to seek clarification and whether either some 
 adjustment to the behaviour could be made or clarification in the docs.

 Here's a boiled down example:

 Validating models with UniqueConstraint usually groups errors by field 
 name:

 class Test(Model):
 test = CharField(max_length=255)

 class Meta:
 constraints = [
 UniqueConstraint(fields=["test"], name="unique"),
 ]

 Test.objects.create(test="abc")
 test = Test(test="abc")
 test.validate_constraints()
 django.core.exceptions.ValidationError: {'test': ['Test with this Test 
 already exists.']}

 However if a condition is added to the UniqueConstraint the validation 
 error is categorised as a non-field error:

 class Test(Model):
 test = CharField(max_length=255)

 class Meta:
 constraints = [
 UniqueConstraint(fields=["test"], name="unique", 
 condition=~Q(test="")),
 ]

 Test.objects.create(test="abc")
 test = Test(test="abc")
 test.validate_constraints()
 django.core.exceptions.ValidationError: {'__all__': ['Constraint 
 “unique” is violated.']}

 This affects the way custom error messages are defined (eg forms 
 error_messages dict vs constraint violation_error_message)

 This behaviour is defined in UniqueConstraint.va

Re: UniqueConstraint validation error message conditional vs non-conditional

2022-09-11 Thread David Sanders
Hi Simon,

Cheers for the explanation.

I'm ok with the error message being the "constraint is violated" generic
message as I agree with what you're saying.

Would it be possible to group the message by field in the same way as
standard unique?

ie, would this be an idea?:

django.core.exceptions.ValidationError: {'test': ['Constraint “unique” is
violated.']}

This way the customised error message would be preserved.

This also leads to another question I had about whether it might be an idea
to allow overriding error messages for constraints in forms using the
error_messages dict… but I'll leave that for another post :)

Regards,
David

On Mon, 12 Sept 2022 at 11:53, charettes  wrote:

> Hello David,
>
> This is expected because Django doesn't have a way to express the
> constraint in words to present to the user when a condition, which could be
> complex, is provided.
>
> When no conditions are defined the metadata is easy to interpret to form a
> sentence out of ("Foo with this field0, field1, fieldN already exists")
> while when a condition is provided Q can define very complex criteria that
> only the developer is able to express in words. This is the same class of
> problem as trying to have a machine describe a text representation of a
> regular expression. It's doable for very simple cases but almost impossible
> for complex ones.
>
> We decided not to try to be clever about what the error message should be
> and opted to have a default that uses the constraint name when a condition
> is present while allowing developers to provide a `violation_error_message`
> at constraint initialization to have them express what a more appropriate
> error message should be.
>
> If we take your particular example, how should Django figure out what the
> error message should be for a UniqueConstraint of this form?
>
> UniqueConstraint(
> fields=["foo"],
> name="unique_uppercase_foo",
> condition=Q(test__upper=F("test"))
> )
>
> The developer who defined the constraint is in a much better position to
> provide an error that says that "Test with an upper case "foo" must be
> unique". If you're not convinced try with a complex condition involving OR
> and mixing transform comparison (e.g. a unique constraint only defined on
> palindromes)
>
> Cheers,
> Simon
>
> Le dimanche 11 septembre 2022 à 14:52:47 UTC-4, othnield...@gmail.com a
> écrit :
>
>> nice one there
>>
>> On Sun, Sep 11, 2022 at 3:36 PM David Sanders 
>> wrote:
>>
>>> Hi folks (and in particular Simon Charette),
>>>
>>> I had a bit of a gotcha moment when a custom unique constraint
>>> validation message disappeared when I added a condition to it. I won't
>>> raise a ticket for this because it looks intentional from the constraint
>>> validation PR, but I wanted to seek clarification and whether either some
>>> adjustment to the behaviour could be made or clarification in the docs.
>>>
>>> Here's a boiled down example:
>>>
>>> Validating models with UniqueConstraint usually groups errors by field
>>> name:
>>>
>>> class Test(Model):
>>> test = CharField(max_length=255)
>>>
>>> class Meta:
>>> constraints = [
>>> UniqueConstraint(fields=["test"], name="unique"),
>>> ]
>>>
>>> Test.objects.create(test="abc")
>>> test = Test(test="abc")
>>> test.validate_constraints()
>>> django.core.exceptions.ValidationError: {'test': ['Test with this Test
>>> already exists.']}
>>>
>>> However if a condition is added to the UniqueConstraint the validation
>>> error is categorised as a non-field error:
>>>
>>> class Test(Model):
>>> test = CharField(max_length=255)
>>>
>>> class Meta:
>>> constraints = [
>>> UniqueConstraint(fields=["test"], name="unique",
>>> condition=~Q(test="")),
>>> ]
>>>
>>> Test.objects.create(test="abc")
>>> test = Test(test="abc")
>>> test.validate_constraints()
>>> django.core.exceptions.ValidationError: {'__all__': ['Constraint
>>> “unique” is violated.']}
>>>
>>> This affects the way custom error messages are defined (eg forms
>>> error_messages dict vs constraint violation_error_message)
>>>
>>> This behaviour is defined in UniqueConstraint.validate() and looks to
>>> be intentional from this advice from Simon:
>>> https://github.com/django/django/pull/14625#issuecomment-897275721
>>>
>>> I was wondering what the rationale for that was? Would it be possible to
>>> make the behaviour consistent? And if not - would a PR to clarify how
>>> ValidationErrors are raised for constraints in the docs be welcomed?
>>>
>>> Cheers,
>>> David
>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups "Django developers (Contributions to Django itself)" group.
>>> To unsubscribe from this group and stop receiving emails from it, send
>>> an email to django-develop...@googlegroups.com.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/django-developers/CADyZw-5w94wvx26OVdSaDHGjKxRGvdnCZYeMOM-p4J9t9JUqMw%40mai

Re: UniqueConstraint validation error message conditional vs non-conditional

2022-09-11 Thread charettes
Hello David,

This is expected because Django doesn't have a way to express the 
constraint in words to present to the user when a condition, which could be 
complex, is provided.

When no conditions are defined the metadata is easy to interpret to form a 
sentence out of ("Foo with this field0, field1, fieldN already exists") 
while when a condition is provided Q can define very complex criteria that 
only the developer is able to express in words. This is the same class of 
problem as trying to have a machine describe a text representation of a 
regular expression. It's doable for very simple cases but almost impossible 
for complex ones.

We decided not to try to be clever about what the error message should be 
and opted to have a default that uses the constraint name when a condition 
is present while allowing developers to provide a `violation_error_message` 
at constraint initialization to have them express what a more appropriate 
error message should be.

If we take your particular example, how should Django figure out what the 
error message should be for a UniqueConstraint of this form?

UniqueConstraint(
fields=["foo"],
name="unique_uppercase_foo",
condition=Q(test__upper=F("test"))
)

The developer who defined the constraint is in a much better position to 
provide an error that says that "Test with an upper case "foo" must be 
unique". If you're not convinced try with a complex condition involving OR 
and mixing transform comparison (e.g. a unique constraint only defined on 
palindromes)

Cheers,
Simon

Le dimanche 11 septembre 2022 à 14:52:47 UTC-4, othnield...@gmail.com a 
écrit :

> nice one there
>
> On Sun, Sep 11, 2022 at 3:36 PM David Sanders  
> wrote:
>
>> Hi folks (and in particular Simon Charette),
>>
>> I had a bit of a gotcha moment when a custom unique constraint validation 
>> message disappeared when I added a condition to it. I won't raise a ticket 
>> for this because it looks intentional from the constraint validation PR, 
>> but I wanted to seek clarification and whether either some adjustment to 
>> the behaviour could be made or clarification in the docs.
>>
>> Here's a boiled down example:
>>
>> Validating models with UniqueConstraint usually groups errors by field 
>> name:
>>
>> class Test(Model):
>> test = CharField(max_length=255)
>>
>> class Meta:
>> constraints = [
>> UniqueConstraint(fields=["test"], name="unique"),
>> ]
>>
>> Test.objects.create(test="abc")
>> test = Test(test="abc")
>> test.validate_constraints()
>> django.core.exceptions.ValidationError: {'test': ['Test with this Test 
>> already exists.']}
>>
>> However if a condition is added to the UniqueConstraint the validation 
>> error is categorised as a non-field error:
>>
>> class Test(Model):
>> test = CharField(max_length=255)
>>
>> class Meta:
>> constraints = [
>> UniqueConstraint(fields=["test"], name="unique", 
>> condition=~Q(test="")),
>> ]
>>
>> Test.objects.create(test="abc")
>> test = Test(test="abc")
>> test.validate_constraints()
>> django.core.exceptions.ValidationError: {'__all__': ['Constraint “unique” 
>> is violated.']}
>>
>> This affects the way custom error messages are defined (eg forms 
>> error_messages dict vs constraint violation_error_message)
>>
>> This behaviour is defined in UniqueConstraint.validate() and looks to be 
>> intentional from this advice from Simon: 
>> https://github.com/django/django/pull/14625#issuecomment-897275721
>>
>> I was wondering what the rationale for that was? Would it be possible to 
>> make the behaviour consistent? And if not - would a PR to clarify how 
>> ValidationErrors are raised for constraints in the docs be welcomed?
>>
>> Cheers,
>> David
>>
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to django-develop...@googlegroups.com.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/django-developers/CADyZw-5w94wvx26OVdSaDHGjKxRGvdnCZYeMOM-p4J9t9JUqMw%40mail.gmail.com
>>  
>> 
>> .
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/455d67d0-ca3e-4bc2-888f-10312dae8915n%40googlegroups.com.


Re: UniqueConstraint validation error message conditional vs non-conditional

2022-09-11 Thread Othniel Davidson
nice one there

On Sun, Sep 11, 2022 at 3:36 PM David Sanders 
wrote:

> Hi folks (and in particular Simon Charette),
>
> I had a bit of a gotcha moment when a custom unique constraint validation
> message disappeared when I added a condition to it. I won't raise a ticket
> for this because it looks intentional from the constraint validation PR,
> but I wanted to seek clarification and whether either some adjustment to
> the behaviour could be made or clarification in the docs.
>
> Here's a boiled down example:
>
> Validating models with UniqueConstraint usually groups errors by field
> name:
>
> class Test(Model):
> test = CharField(max_length=255)
>
> class Meta:
> constraints = [
> UniqueConstraint(fields=["test"], name="unique"),
> ]
>
> Test.objects.create(test="abc")
> test = Test(test="abc")
> test.validate_constraints()
> django.core.exceptions.ValidationError: {'test': ['Test with this Test
> already exists.']}
>
> However if a condition is added to the UniqueConstraint the validation
> error is categorised as a non-field error:
>
> class Test(Model):
> test = CharField(max_length=255)
>
> class Meta:
> constraints = [
> UniqueConstraint(fields=["test"], name="unique",
> condition=~Q(test="")),
> ]
>
> Test.objects.create(test="abc")
> test = Test(test="abc")
> test.validate_constraints()
> django.core.exceptions.ValidationError: {'__all__': ['Constraint “unique”
> is violated.']}
>
> This affects the way custom error messages are defined (eg forms
> error_messages dict vs constraint violation_error_message)
>
> This behaviour is defined in UniqueConstraint.validate() and looks to be
> intentional from this advice from Simon:
> https://github.com/django/django/pull/14625#issuecomment-897275721
>
> I was wondering what the rationale for that was? Would it be possible to
> make the behaviour consistent? And if not - would a PR to clarify how
> ValidationErrors are raised for constraints in the docs be welcomed?
>
> Cheers,
> David
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/CADyZw-5w94wvx26OVdSaDHGjKxRGvdnCZYeMOM-p4J9t9JUqMw%40mail.gmail.com
> 
> .
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAPRrvgzm5O110ZRLD9B6YO-kF%2B61tCEmBc-E0z02pCKbeaGJcg%40mail.gmail.com.