Refs #28458 -- Added validate_choices to ModelMultipleChoiceField. #14033

gasparb16 · 2021-02-22T16:10:54Z

I fixed the review findings from: #7181

Co-authored-by: blueyed github@thequod.de

felixxm

@gasparb16 Thanks for this patch 👍

felixxm · 2021-02-25T09:21:00Z

docs/ref/forms/fields.txt

@@ -1357,6 +1357,15 @@ generating choices. See :ref:`iterating-relationship-choices` for details.

 .. _iterating-relationship-choices:

+.. method:: ModelMultipleChoiceField.validate_choices(queryset, field_name, selected_choices)


This should be above the .. _iterating-relationship-choices:. Also versionadded:: 4.0 annotation and release notes are missing, see #7181 (comment).

I was not sure what the new use-case would be regarding the super() call, I would gladly accept some help with this.

We should have a valid use case if we want to add a new hook 🤔 It can be used to avoid a queryset evaluation, but I'm not sure it's enough to justify adding it. It's really niche and was more of a workaround than a real need (see ticket-27148). I'm going to say wontfix unless there's a more legitimate use-case.

Yeah I understand, I just wanted to fix the issues in the PR and code some and if it is needed merge it. But I am fine with a wontfix that closes the ticket as well.

felixxm · 2021-02-25T09:23:00Z

tests/model_forms/tests.py

+
+        with self.assertNumQueries(1):
+            field = CustomModelMultipleChoiceField(Category.objects.all())
+            with self.assertRaises(ValidationError) as exc:


Use assertRaisesMessage().

felixxm · 2021-02-25T09:23:15Z

tests/model_forms/tests.py

+    def test_model_multiple_choice_field_validate_choices(self):
+        with self.assertNumQueries(1):
+            field = forms.ModelMultipleChoiceField(Category.objects.all())
+            with self.assertRaises(ValidationError) as exc:


Use assertRaisesMessage().

I am not sure if we can use this, because I would like to assert to the exc.exception.params not the message

True, exc -> cm.

felixxm · 2021-02-25T09:30:39Z

tests/model_forms/tests.py

+        class CustomModelMultipleChoiceField(forms.ModelMultipleChoiceField, TestCase):
+            def validate_choices(self, queryset, field_name, selected_choices):
+                self.assertIsInstance(queryset, models.QuerySet)
+                self.assertQuerysetEqual(queryset.order_by('id'), [1], lambda a: a.id)


We cannot make serial pk assumption:

diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index 8b54611010..ea68a105d6 100644 --- a/tests/model_forms/tests.py +++ b/tests/model_forms/tests.py @@ -1765,10 +1765,12 @@ class ModelMultipleChoiceFieldTests(TestCase): f.clean([c6.id]) def test_model_multiple_choice_field_validate_choices_called_properly(self): + c1 = self.c1 + class CustomModelMultipleChoiceField(forms.ModelMultipleChoiceField, TestCase): def validate_choices(self, queryset, field_name, selected_choices): self.assertIsInstance(queryset, models.QuerySet) - self.assertQuerysetEqual(queryset.order_by('id'), [1], lambda a: a.id) + self.assertSequenceEqual(queryset, [c1]) self.assertIsInstance(field_name, str) self.assertEqual(field_name, 'pk') self.assertIsInstance(selected_choices, frozenset)

felixxm · 2021-02-25T09:31:47Z

docs/ref/forms/fields.txt

@@ -1357,6 +1357,15 @@ generating choices. See :ref:`iterating-relationship-choices` for details.

 .. _iterating-relationship-choices:

+.. method:: ModelMultipleChoiceField.validate_choices(queryset, field_name, selected_choices)
+
+``validate_choices`` gets called with the filtered queryset (``queryset``), the name


Suggested change

``validate_choices`` gets called with the filtered queryset (``queryset``), the name

``validate_choices`` gets called with the filtered ``queryset``, the name

felixxm · 2021-02-25T09:31:56Z

docs/ref/forms/fields.txt

+.. method:: ModelMultipleChoiceField.validate_choices(queryset, field_name, selected_choices)
+
+``validate_choices`` gets called with the filtered queryset (``queryset``), the name
+of the field, and the list of selected values.  It should return the queryset.


Suggested change

of the field, and the list of selected values. It should return the queryset.

of the field, and the list of selected values. It should return the queryset.

felixxm · 2021-02-25T09:31:59Z

docs/ref/forms/fields.txt

+of the field, and the list of selected values.  It should return the queryset.
+
+The default implementation raises a ``ValidationError`` in case any of the
+``values`` is not in the list of attribute lookups of ``field_name`` in ``qs``


`

Suggested change

``values`` is not in the list of attribute lookups of ``field_name`` in ``qs``

``values`` is not in the list of attribute lookups of ``field_name`` in ``queryset``

Co-authored-by: blueyed <github@thequod.de>

felixxm · 2021-03-02T08:51:27Z

@gasparb16 Thanks for your effort! 👍

Closing per ticket.

recepsirin approved these changes Feb 22, 2021

View reviewed changes

felixxm reviewed Feb 25, 2021

View reviewed changes

gasparb16 force-pushed the gaspar-28458 branch from df65fef to 2137daf Compare February 26, 2021 08:06

Refs #28458 -- Added validate_choices to ModelMultipleChoiceField.

01285bd

Co-authored-by: blueyed <github@thequod.de>

gasparb16 force-pushed the gaspar-28458 branch from 2137daf to 01285bd Compare February 26, 2021 10:44

felixxm closed this Mar 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refs #28458 -- Added validate_choices to ModelMultipleChoiceField. #14033

Refs #28458 -- Added validate_choices to ModelMultipleChoiceField. #14033

gasparb16 commented Feb 22, 2021

felixxm left a comment

felixxm Feb 25, 2021

gasparb16 Feb 25, 2021

felixxm Mar 1, 2021

gasparb16 Mar 2, 2021

felixxm Feb 25, 2021

felixxm Feb 25, 2021

gasparb16 Feb 25, 2021

felixxm Feb 25, 2021

felixxm Feb 25, 2021

felixxm Feb 25, 2021

felixxm Feb 25, 2021

felixxm Feb 25, 2021

felixxm commented Mar 2, 2021

		@@ -1357,6 +1357,15 @@ generating choices. See :ref:`iterating-relationship-choices` for details.

		.. _iterating-relationship-choices:

		.. method:: ModelMultipleChoiceField.validate_choices(queryset, field_name, selected_choices)

	``validate_choices`` gets called with the filtered queryset (``queryset``), the name
	``validate_choices`` gets called with the filtered ``queryset``, the name

	of the field, and the list of selected values. It should return the queryset.
	of the field, and the list of selected values. It should return the queryset.

	``values`` is not in the list of attribute lookups of ``field_name`` in ``qs``
	``values`` is not in the list of attribute lookups of ``field_name`` in ``queryset``

Refs #28458 -- Added validate_choices to ModelMultipleChoiceField. #14033

Refs #28458 -- Added validate_choices to ModelMultipleChoiceField. #14033

Conversation

gasparb16 commented Feb 22, 2021

felixxm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

felixxm commented Mar 2, 2021