From 7e8962c6227a2b78f290b19e8917b39d009102b9 Mon Sep 17 00:00:00 2001 From: Genaro Camele Date: Thu, 25 Sep 2025 22:35:14 -0300 Subject: [PATCH 1/8] Added validation on ManyToMany relations when default=None + Added tests --- rest_framework/serializers.py | 8 ++++++++ tests/test_serializer.py | 37 ++++++++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 8fe284bc84..74dd34be3a 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -1090,6 +1090,14 @@ def get_fields(self): # Determine the fields that should be included on the serializer. fields = {} + # If it's a ManyToMany field, and the default is None, then raises an exception to prevent exceptions on .set() + for field_name in declared_fields.keys(): + if field_name in info.relations and info.relations[field_name].to_many and declared_fields[field_name].default is None: + raise ValueError( + f"The field '{field_name}' on serializer '{self.__class__.__name__}' is a ManyToMany field and cannot have a default value of None. " + "Please set an appropriate default value, such as an empty list, or remove the default." + ) + for field_name in field_names: # If the field is explicitly declared on the class then use that. if field_name in declared_fields: diff --git a/tests/test_serializer.py b/tests/test_serializer.py index cefa2ee38a..960c663f64 100644 --- a/tests/test_serializer.py +++ b/tests/test_serializer.py @@ -6,12 +6,14 @@ import pytest from django.db import models +from django.test import TestCase from rest_framework import exceptions, fields, relations, serializers from rest_framework.fields import Field from .models import ( - ForeignKeyTarget, NestedForeignKeySource, NullableForeignKeySource + ForeignKeyTarget, ManyToManySource, ManyToManyTarget, + NestedForeignKeySource, NullableForeignKeySource ) from .utils import MockObject @@ -64,6 +66,7 @@ def setup_method(self): class ExampleSerializer(serializers.Serializer): char = serializers.CharField() integer = serializers.IntegerField() + self.Serializer = ExampleSerializer def test_valid_serializer(self): @@ -774,3 +777,35 @@ def test_nested_key(self): ret = {'a': 1} self.s.set_value(ret, ['x', 'y'], 2) assert ret == {'a': 1, 'x': {'y': 2}} + + +class TestWarningManyToMany(TestCase): + def test_warning_many_to_many(self): + """Tests that using a PrimaryKeyRelatedField for a ManyToMany field breaks with default=None.""" + class ManyToManySourceSerializer(serializers.ModelSerializer): + targets = serializers.PrimaryKeyRelatedField( + many=True, + queryset=ManyToManyTarget.objects.all(), + default=None + ) + + class Meta: + model = ManyToManySource + fields = '__all__' + + # Instantiates with invalid data (not value for a ManyToMany field to force using the default) + serializer = ManyToManySourceSerializer(data={ + "name": "Invalid Example", + }) + + error_msg = "The field 'targets' on serializer 'ManyToManySourceSerializer' is a ManyToMany field and cannot have a default value of None. Please set an appropriate default value, such as an empty list, or remove the default." + + # Calls to get_fields() should raise a ValueError + with pytest.raises(ValueError) as exc_info: + serializer.get_fields() + assert str(exc_info.value) == error_msg + + # Calls to is_valid() should behave the same + with pytest.raises(ValueError) as exc_info: + serializer.is_valid(raise_exception=True) + assert str(exc_info.value) == error_msg From edaf07b45db92429919f80ab690aaa14c2fe4b51 Mon Sep 17 00:00:00 2001 From: Genaro Camele Date: Thu, 25 Sep 2025 22:35:31 -0300 Subject: [PATCH 2/8] Added some important notes on contributing.md --- docs/community/contributing.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/community/contributing.md b/docs/community/contributing.md index aceff45ac2..15b840877a 100644 --- a/docs/community/contributing.md +++ b/docs/community/contributing.md @@ -81,6 +81,23 @@ To run the tests, clone the repository, and then: # Run the tests ./runtests.py +--- + +**Note:** +If your tests require access to the database, do not forget to inherit from `django.test.TestCase`. +For example: + + from django.test import TestCase + + class MyDatabaseTest(TestCase): + def test_something(self): + # Your test code here + pass + +You can reuse existing models defined in `tests/models.py` for your tests. + +--- + ### Test options Run using a more concise output style. @@ -99,6 +116,10 @@ Shorter form to run the tests for a given test method. ./runtests.py test_this_method +**Note:** If you do not want the output to be captured (for example, to see print statements directly), you can use the `-s` flag: + + ./runtests.py -s + Note: The test case and test method matching is fuzzy and will sometimes run other tests that contain a partial string match to the given command line input. ### Running against multiple environments From 04703dca8a5b87b2d5ba33b9bdaf9ec7be4c2058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Asif=20Saif=20Uddin=20=7B=22Auvi=22=3A=22=E0=A6=85?= =?UTF-8?q?=E0=A6=AD=E0=A6=BF=22=7D?= Date: Sun, 28 Sep 2025 10:09:37 +0600 Subject: [PATCH 3/8] Update tests/test_serializer.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- tests/test_serializer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_serializer.py b/tests/test_serializer.py index 960c663f64..5dfbeb9356 100644 --- a/tests/test_serializer.py +++ b/tests/test_serializer.py @@ -793,7 +793,7 @@ class Meta: model = ManyToManySource fields = '__all__' - # Instantiates with invalid data (not value for a ManyToMany field to force using the default) + # Instantiates with invalid data (no value for a ManyToMany field to force using the default) serializer = ManyToManySourceSerializer(data={ "name": "Invalid Example", }) From 9d8b176374e32da2a8695be4c5efe4e51421e1a2 Mon Sep 17 00:00:00 2001 From: Genaro Camele Date: Mon, 13 Oct 2025 16:29:24 -0300 Subject: [PATCH 4/8] Improved ValueError msg --- rest_framework/serializers.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 74dd34be3a..c655f47a92 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -1094,8 +1094,7 @@ def get_fields(self): for field_name in declared_fields.keys(): if field_name in info.relations and info.relations[field_name].to_many and declared_fields[field_name].default is None: raise ValueError( - f"The field '{field_name}' on serializer '{self.__class__.__name__}' is a ManyToMany field and cannot have a default value of None. " - "Please set an appropriate default value, such as an empty list, or remove the default." + f"The field '{field_name}' on serializer '{self.__class__.__name__}' is a ManyToMany field and cannot have a default value of None." ) for field_name in field_names: From e87a82b3fc72e1cd149b6a0f13b3763364c3e2d7 Mon Sep 17 00:00:00 2001 From: Genaro Camele Date: Mon, 13 Oct 2025 16:32:33 -0300 Subject: [PATCH 5/8] Improved comment on serializer instantiation --- tests/test_serializer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_serializer.py b/tests/test_serializer.py index 5dfbeb9356..4973c3f2c6 100644 --- a/tests/test_serializer.py +++ b/tests/test_serializer.py @@ -793,7 +793,7 @@ class Meta: model = ManyToManySource fields = '__all__' - # Instantiates with invalid data (no value for a ManyToMany field to force using the default) + # Instantiates serializer without 'value' field to force using the default=None for the ManyToMany relation serializer = ManyToManySourceSerializer(data={ "name": "Invalid Example", }) From eb4ed07d943e98bc336075a32020c990bd5dbd0c Mon Sep 17 00:00:00 2001 From: Genaro Camele Date: Mon, 13 Oct 2025 16:36:35 -0300 Subject: [PATCH 6/8] Refine error message in ManyToManySourceSerializer test Updated the error message for ManyToManySourceSerializer to remove suggestion for default value to make the test work --- tests/test_serializer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_serializer.py b/tests/test_serializer.py index 4973c3f2c6..ed8a749118 100644 --- a/tests/test_serializer.py +++ b/tests/test_serializer.py @@ -798,7 +798,7 @@ class Meta: "name": "Invalid Example", }) - error_msg = "The field 'targets' on serializer 'ManyToManySourceSerializer' is a ManyToMany field and cannot have a default value of None. Please set an appropriate default value, such as an empty list, or remove the default." + error_msg = "The field 'targets' on serializer 'ManyToManySourceSerializer' is a ManyToMany field and cannot have a default value of None." # Calls to get_fields() should raise a ValueError with pytest.raises(ValueError) as exc_info: From 9da97c90f81f3dc9ff71d17bc52e747ed3757756 Mon Sep 17 00:00:00 2001 From: Genaro Camele Date: Mon, 13 Oct 2025 21:13:55 -0300 Subject: [PATCH 7/8] Add clarification on `-s` flag usage right after the -q tip --- docs/community/contributing.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/community/contributing.md b/docs/community/contributing.md index 15b840877a..5be0072db6 100644 --- a/docs/community/contributing.md +++ b/docs/community/contributing.md @@ -104,6 +104,12 @@ Run using a more concise output style. ./runtests.py -q + +If you do not want the output to be captured (for example, to see print statements directly), you can use the `-s` flag. + + ./runtests.py -s + + Run the tests for a given test case. ./runtests.py MyTestCase @@ -116,9 +122,6 @@ Shorter form to run the tests for a given test method. ./runtests.py test_this_method -**Note:** If you do not want the output to be captured (for example, to see print statements directly), you can use the `-s` flag: - - ./runtests.py -s Note: The test case and test method matching is fuzzy and will sometimes run other tests that contain a partial string match to the given command line input. From 542d4dc388e81c02d5ed4699985d2e93c1a76f2a Mon Sep 17 00:00:00 2001 From: Genaro Camele Date: Mon, 13 Oct 2025 21:18:52 -0300 Subject: [PATCH 8/8] Update contributing guidelines to include pytest decorator for database tests --- docs/community/contributing.md | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/docs/community/contributing.md b/docs/community/contributing.md index 5be0072db6..797bf72e38 100644 --- a/docs/community/contributing.md +++ b/docs/community/contributing.md @@ -83,9 +83,9 @@ To run the tests, clone the repository, and then: --- -**Note:** -If your tests require access to the database, do not forget to inherit from `django.test.TestCase`. -For example: +**Note:** if your tests require access to the database, do not forget to inherit from `django.test.TestCase` or use the `@pytest.mark.django_db()` decorator. + +For example, with TestCase: from django.test import TestCase @@ -94,6 +94,16 @@ For example: # Your test code here pass +Or with decorator: + + import pytest + + @pytest.mark.django_db() + class MyDatabaseTest: + def test_something(self): + # Your test code here + pass + You can reuse existing models defined in `tests/models.py` for your tests. ---