Repository: graphql-python/graphene-django

Issue:
Convert DecimalField to String rather than Float
DecimalField entries are currently being converted to Float (https://github.com/graphql-python/graphene-django/blob/8f5a425859ea380d0640c441c5c7f3b1acae17b4/graphene_django/form_converter.py#L56-L59)

Full support for Decimal would be great, but in the meantime I propose that DecimalField should be converted to String instead. Once you've converted a Decimal to a Float, you've probably lost the benefit of why you used Decimal in the first place, since you'll now have values like `3.14000000000000012`, rather than `3.14`. If it is converted to String, it is simple enough for the user to convert that String back to Decimal, whereas converting from Float to Decimal is not straightforward.

Discussion:
zbyte64: #91 is fixed, is this still relevant? What's this about `test_should_query_filter_node_limit` ?
ulgens: @zbyte64 I don't think this is still relevant, i'll test and update it asap. The issue with the test, as far as i remember, registering a decimal as string affected incoming data type in pagination, paginator was expecting a decimal value (not sure how and why).
ulgens: @zbyte64 This is still relevant. #1083 doesn't touch Django form based conversions at all. I'll try to make it work with that new `graphene.Decimal` type.

Patch:
diff --git a/graphene_django/filter/__init__.py b/graphene_django/filter/__init__.py
index daafe56..5de36ad 100644
--- a/graphene_django/filter/__init__.py
+++ b/graphene_django/filter/__init__.py
@@ -9,7 +9,7 @@ if not DJANGO_FILTER_INSTALLED:
     )
 else:
     from .fields import DjangoFilterConnectionField
-    from .filterset import GlobalIDFilter, GlobalIDMultipleChoiceFilter
+    from .filters import GlobalIDFilter, GlobalIDMultipleChoiceFilter
 
     __all__ = [
         "DjangoFilterConnectionField",
diff --git a/graphene_django/filter/filters.py b/graphene_django/filter/filters.py
new file mode 100644
index 0000000..44832b5
--- /dev/null
+++ b/graphene_django/filter/filters.py
@@ -0,0 +1,75 @@
+from django.core.exceptions import ValidationError
+from django.forms import Field
+
+from django_filters import Filter, MultipleChoiceFilter
+
+from graphql_relay.node.node import from_global_id
+
+from ..forms import GlobalIDFormField, GlobalIDMultipleChoiceField
+
+
+class GlobalIDFilter(Filter):
+    """
+    Filter for Relay global ID.
+    """
+
+    field_class = GlobalIDFormField
+
+    def filter(self, qs, value):
+        """ Convert the filter value to a primary key before filtering """
+        _id = None
+        if value is not None:
+            _, _id = from_global_id(value)
+        return super(GlobalIDFilter, self).filter(qs, _id)
+
+
+class GlobalIDMultipleChoiceFilter(MultipleChoiceFilter):
+    field_class = GlobalIDMultipleChoiceField
+
+    def filter(self, qs, value):
+        gids = [from_global_id(v)[1] for v in value]
+        return super(GlobalIDMultipleChoiceFilter, self).filter(qs, gids)
+
+
+class InFilter(Filter):
+    """
+    Filter for a list of value using the `__in` Django filter.
+    """
+
+    def filter(self, qs, value):
+        """
+        Override the default filter class to check first weather the list is
+        empty or not.
+        This needs to be done as in this case we expect to get an empty output
+        (if not an exclude filter) but django_filter consider an empty list
+        to be an empty input value (see `EMPTY_VALUES`) meaning that
+        the filter does not need to be applied (hence returning the original
+        queryset).
+        """
+        if value is not None and len(value) == 0:
+            if self.exclude:
+                return qs
+            else:
+                return qs.none()
+        else:
+            return super(InFilter, self).filter(qs, value)
+
+
+def validate_range(value):
+    """
+    Validator for range filter input: the list of value must be of length 2.
+    Note that validators are only run if the value is not empty.
+    """
+    if len(value) != 2:
+        raise ValidationError(
+            "Invalid range specified: it needs to contain 2 values.", code="invalid"
+        )
+
+
+class RangeField(Field):
+    default_validators = [validate_range]
+    empty_values = [None]
+
+
+class RangeFilter(Filter):
+    field_class = RangeField
diff --git a/graphene_django/filter/filterset.py b/graphene_django/filter/filterset.py
index 7676ea8..0fd0a82 100644
--- a/graphene_django/filter/filterset.py
+++ b/graphene_django/filter/filterset.py
@@ -1,32 +1,11 @@
 import itertools
 
 from django.db import models
-from django_filters import Filter, MultipleChoiceFilter, VERSION
+from django_filters import VERSION
 from django_filters.filterset import BaseFilterSet, FilterSet
 from django_filters.filterset import FILTER_FOR_DBFIELD_DEFAULTS
 
-from graphql_relay.node.node import from_global_id
-
-from ..forms import GlobalIDFormField, GlobalIDMultipleChoiceField
-
-
-class GlobalIDFilter(Filter):
-    field_class = GlobalIDFormField
-
-    def filter(self, qs, value):
-        """ Convert the filter value to a primary key before filtering """
-        _id = None
-        if value is not None:
-            _, _id = from_global_id(value)
-        return super(GlobalIDFilter, self).filter(qs, _id)
-
-
-class GlobalIDMultipleChoiceFilter(MultipleChoiceFilter):
-    field_class = GlobalIDMultipleChoiceField
-
-    def filter(self, qs, value):
-        gids = [from_global_id(v)[1] for v in value]
-        return super(GlobalIDMultipleChoiceFilter, self).filter(qs, gids)
+from .filters import GlobalIDFilter, GlobalIDMultipleChoiceFilter
 
 
 GRAPHENE_FILTER_SET_OVERRIDES = {
diff --git a/graphene_django/filter/utils.py b/graphene_django/filter/utils.py
index 71c5b49..dce08c7 100644
--- a/graphene_django/filter/utils.py
+++ b/graphene_django/filter/utils.py
@@ -6,6 +6,7 @@ from django_filters.utils import get_model_field
 from django_filters.filters import Filter, BaseCSVFilter
 
 from .filterset import custom_filterset_factory, setup_filterset
+from .filters import InFilter, RangeFilter
 
 
 def get_filtering_args_from_filterset(filterset_class, type):
@@ -80,9 +81,20 @@ def replace_csv_filters(filterset_class):
     """
     for name, filter_field in six.iteritems(filterset_class.base_filters):
         filter_type = filter_field.lookup_expr
-        if filter_type in ["in", "range"]:
+        if filter_type == "in":
+            assert isinstance(filter_field, BaseCSVFilter)
+            filterset_class.base_filters[name] = InFilter(
+                field_name=filter_field.field_name,
+                lookup_expr=filter_field.lookup_expr,
+                label=filter_field.label,
+                method=filter_field.method,
+                exclude=filter_field.exclude,
+                **filter_field.extra
+            )
+
+        if filter_type == "range":
             assert isinstance(filter_field, BaseCSVFilter)
-            filterset_class.base_filters[name] = Filter(
+            filterset_class.base_filters[name] = RangeFilter(
                 field_name=filter_field.field_name,
                 lookup_expr=filter_field.lookup_expr,
                 label=filter_field.label,
diff --git a/graphene_django/forms/converter.py b/graphene_django/forms/converter.py
index 5d17680..9db0a77 100644
--- a/graphene_django/forms/converter.py
+++ b/graphene_django/forms/converter.py
@@ -1,12 +1,23 @@
 from django import forms
 from django.core.exceptions import ImproperlyConfigured
 
-from graphene import ID, Boolean, Float, Int, List, String, UUID, Date, DateTime, Time
+from graphene import (
+    Boolean,
+    Date,
+    DateTime,
+    Decimal,
+    Float,
+    ID,
+    Int,
+    List,
+    String,
+    Time,
+    UUID,
+)
 
 from .forms import GlobalIDFormField, GlobalIDMultipleChoiceField
 from ..utils import import_single_dispatch
 
-
 singledispatch = import_single_dispatch()
 
 
@@ -52,6 +63,10 @@ def convert_form_field_to_nullboolean(field):
 
 
 @convert_form_field.register(forms.DecimalField)
+def convert_field_to_decimal(field):
+    return Decimal(description=field.help_text, required=field.required)
+
+
 @convert_form_field.register(forms.FloatField)
 def convert_form_field_to_float(field):
     return Float(description=field.help_text, required=field.required)
diff --git a/graphene_django/rest_framework/mutation.py b/graphene_django/rest_framework/mutation.py
index 000b21e..9e2ae12 100644
--- a/graphene_django/rest_framework/mutation.py
+++ b/graphene_django/rest_framework/mutation.py
@@ -18,6 +18,7 @@ class SerializerMutationOptions(MutationOptions):
     model_class = None
     model_operations = ["create", "update"]
     serializer_class = None
+    optional_fields = ()
 
 
 def fields_for_serializer(
@@ -27,6 +28,7 @@ def fields_for_serializer(
     is_input=False,
     convert_choices_to_enum=True,
     lookup_field=None,
+    optional_fields=(),
 ):
     fields = OrderedDict()
     for name, field in serializer.fields.items():
@@ -44,9 +46,13 @@ def fields_for_serializer(
 
         if is_not_in_only or is_excluded:
             continue
+        is_optional = name in optional_fields
 
         fields[name] = convert_serializer_field(
-            field, is_input=is_input, convert_choices_to_enum=convert_choices_to_enum
+            field,
+            is_input=is_input,
+            convert_choices_to_enum=convert_choices_to_enum,
+            force_optional=is_optional,
         )
     return fields
 
@@ -70,6 +76,7 @@ class SerializerMutation(ClientIDMutation):
         exclude_fields=(),
         convert_choices_to_enum=True,
         _meta=None,
+        optional_fields=(),
         **options
     ):
 
@@ -95,6 +102,7 @@ class SerializerMutation(ClientIDMutation):
             is_input=True,
             convert_choices_to_enum=convert_choices_to_enum,
             lookup_field=lookup_field,
+            optional_fields=optional_fields,
         )
         output_fields = fields_for_serializer(
             serializer,
diff --git a/graphene_django/rest_framework/serializer_converter.py b/graphene_django/rest_framework/serializer_converter.py
index 82a113a..2535fe7 100644
--- a/graphene_django/rest_framework/serializer_converter.py
+++ b/graphene_django/rest_framework/serializer_converter.py
@@ -19,7 +19,9 @@ def get_graphene_type_from_serializer_field(field):
     )
 
 
-def convert_serializer_field(field, is_input=True, convert_choices_to_enum=True):
+def convert_serializer_field(
+    field, is_input=True, convert_choices_to_enum=True, force_optional=False
+):
     """
     Converts a django rest frameworks field to a graphql field
     and marks the field as required if we are creating an input type
@@ -32,7 +34,10 @@ def convert_serializer_field(field, is_input=True, convert_choices_to_enum=True)
         graphql_type = get_graphene_type_from_serializer_field(field)
 
     args = []
-    kwargs = {"description": field.help_text, "required": is_input and field.required}
+    kwargs = {
+        "description": field.help_text,
+        "required": is_input and field.required and not force_optional,
+    }
 
     # if it is a tuple or a list it means that we are returning
     # the graphql type and the child type
@@ -110,8 +115,12 @@ def convert_serializer_field_to_bool(field):
     return graphene.Boolean
 
 
-@get_graphene_type_from_serializer_field.register(serializers.FloatField)
 @get_graphene_type_from_serializer_field.register(serializers.DecimalField)
+def convert_serializer_field_to_decimal(field):
+    return graphene.Decimal
+
+
+@get_graphene_type_from_serializer_field.register(serializers.FloatField)
 def convert_serializer_field_to_float(field):
     return graphene.Float
 


Test patch:
diff --git a/graphene_django/filter/tests/test_fields.py b/graphene_django/filter/tests/test_fields.py
index 18e7f0c..6de8361 100644
--- a/graphene_django/filter/tests/test_fields.py
+++ b/graphene_django/filter/tests/test_fields.py
@@ -5,7 +5,7 @@ import pytest
 from django.db.models import TextField, Value
 from django.db.models.functions import Concat
 
-from graphene import Argument, Boolean, Field, Float, ObjectType, Schema, String
+from graphene import Argument, Boolean, Decimal, Field, ObjectType, Schema, String
 from graphene.relay import Node
 from graphene_django import DjangoObjectType
 from graphene_django.forms import GlobalIDFormField, GlobalIDMultipleChoiceField
@@ -388,7 +388,7 @@ def test_filterset_descriptions():
     field = DjangoFilterConnectionField(ArticleNode, filterset_class=ArticleIdFilter)
     max_time = field.args["max_time"]
     assert isinstance(max_time, Argument)
-    assert max_time.type == Float
+    assert max_time.type == Decimal
     assert max_time.description == "The maximum time"
 
 
@@ -671,7 +671,7 @@ def test_should_query_filter_node_limit():
     schema = Schema(query=Query)
     query = """
         query NodeFilteringQuery {
-            allReporters(limit: 1) {
+            allReporters(limit: "1") {
                 edges {
                     node {
                         id
diff --git a/graphene_django/filter/tests/test_in_filter.py b/graphene_django/filter/tests/test_in_filter.py
index 7bbee65..9e9c323 100644
--- a/graphene_django/filter/tests/test_in_filter.py
+++ b/graphene_django/filter/tests/test_in_filter.py
@@ -157,20 +157,19 @@ def test_int_in_filter():
     ]
 
 
-def test_int_range_filter():
+def test_in_filter_with_empty_list():
     """
-    Test in filter on an integer field.
+    Check that using a in filter with an empty list provided as input returns no objects.
     """
     Pet.objects.create(name="Brutus", age=12)
     Pet.objects.create(name="Mimi", age=8)
-    Pet.objects.create(name="Jojo, the rabbit", age=3)
     Pet.objects.create(name="Picotin", age=5)
 
     schema = Schema(query=Query)
 
     query = """
     query {
-        pets (age_Range: [4, 9]) {
+        pets (name_In: []) {
             edges {
                 node {
                     name
@@ -181,7 +180,4 @@ def test_int_range_filter():
     """
     result = schema.execute(query)
     assert not result.errors
-    assert result.data["pets"]["edges"] == [
-        {"node": {"name": "Mimi"}},
-        {"node": {"name": "Picotin"}},
-    ]
+    assert len(result.data["pets"]["edges"]) == 0
diff --git a/graphene_django/filter/tests/test_range_filter.py b/graphene_django/filter/tests/test_range_filter.py
new file mode 100644
index 0000000..4d8db4f
--- /dev/null
+++ b/graphene_django/filter/tests/test_range_filter.py
@@ -0,0 +1,115 @@
+import ast
+import json
+import pytest
+
+from django_filters import FilterSet
+from django_filters import rest_framework as filters
+from graphene import ObjectType, Schema
+from graphene.relay import Node
+from graphene_django import DjangoObjectType
+from graphene_django.tests.models import Pet
+from graphene_django.utils import DJANGO_FILTER_INSTALLED
+
+pytestmark = []
+
+if DJANGO_FILTER_INSTALLED:
+    from graphene_django.filter import DjangoFilterConnectionField
+else:
+    pytestmark.append(
+        pytest.mark.skipif(
+            True, reason="django_filters not installed or not compatible"
+        )
+    )
+
+
+class PetNode(DjangoObjectType):
+    class Meta:
+        model = Pet
+        interfaces = (Node,)
+        filter_fields = {
+            "name": ["exact", "in"],
+            "age": ["exact", "in", "range"],
+        }
+
+
+class Query(ObjectType):
+    pets = DjangoFilterConnectionField(PetNode)
+
+
+def test_int_range_filter():
+    """
+    Test range filter on an integer field.
+    """
+    Pet.objects.create(name="Brutus", age=12)
+    Pet.objects.create(name="Mimi", age=8)
+    Pet.objects.create(name="Jojo, the rabbit", age=3)
+    Pet.objects.create(name="Picotin", age=5)
+
+    schema = Schema(query=Query)
+
+    query = """
+    query {
+        pets (age_Range: [4, 9]) {
+            edges {
+                node {
+                    name
+                }
+            }
+        }
+    }
+    """
+    result = schema.execute(query)
+    assert not result.errors
+    assert result.data["pets"]["edges"] == [
+        {"node": {"name": "Mimi"}},
+        {"node": {"name": "Picotin"}},
+    ]
+
+
+def test_range_filter_with_invalid_input():
+    """
+    Test range filter used with invalid inputs raise an error.
+    """
+    Pet.objects.create(name="Brutus", age=12)
+    Pet.objects.create(name="Mimi", age=8)
+    Pet.objects.create(name="Jojo, the rabbit", age=3)
+    Pet.objects.create(name="Picotin", age=5)
+
+    schema = Schema(query=Query)
+
+    query = """
+    query ($rangeValue: [Int]) {
+        pets (age_Range: $rangeValue) {
+            edges {
+                node {
+                    name
+                }
+            }
+        }
+    }
+    """
+    expected_error = json.dumps(
+        {
+            "age__range": [
+                {
+                    "message": "Invalid range specified: it needs to contain 2 values.",
+                    "code": "invalid",
+                }
+            ]
+        }
+    )
+
+    # Empty list
+    result = schema.execute(query, variables={"rangeValue": []})
+    assert len(result.errors) == 1
+    assert ast.literal_eval(result.errors[0].message)[0] == expected_error
+
+    # Only one item in the list
+    result = schema.execute(query, variables={"rangeValue": [1]})
+    assert len(result.errors) == 1
+    assert ast.literal_eval(result.errors[0].message)[0] == expected_error
+
+    # More than 2 items in the list
+    result = schema.execute(query, variables={"rangeValue": [1, 2, 3]})
+    assert len(result.errors) == 1
+    assert ast.literal_eval(result.errors[0].message)[0] == expected_error
diff --git a/graphene_django/forms/tests/test_converter.py b/graphene_django/forms/tests/test_converter.py
index ccf630f..78b315c 100644
--- a/graphene_django/forms/tests/test_converter.py
+++ b/graphene_django/forms/tests/test_converter.py
@@ -1,19 +1,19 @@
 from django import forms
 from py.test import raises
 
-import graphene
 from graphene import (
-    String,
-    Int,
     Boolean,
+    Date,
+    DateTime,
+    Decimal,
     Float,
     ID,
-    UUID,
+    Int,
     List,
     NonNull,
-    DateTime,
-    Date,
+    String,
     Time,
+    UUID,
 )
 
 from ..converter import convert_form_field
@@ -97,8 +97,8 @@ def test_should_float_convert_float():
     assert_conversion(forms.FloatField, Float)
 
 
-def test_should_decimal_convert_float():
-    assert_conversion(forms.DecimalField, Float)
+def test_should_decimal_convert_decimal():
+    assert_conversion(forms.DecimalField, Decimal)
 
 
 def test_should_multiple_choice_convert_list():
diff --git a/graphene_django/rest_framework/tests/test_field_converter.py b/graphene_django/rest_framework/tests/test_field_converter.py
index daa8349..4858365 100644
--- a/graphene_django/rest_framework/tests/test_field_converter.py
+++ b/graphene_django/rest_framework/tests/test_field_converter.py
@@ -133,9 +133,9 @@ def test_should_float_convert_float():
     assert_conversion(serializers.FloatField, graphene.Float)
 
 
-def test_should_decimal_convert_float():
+def test_should_decimal_convert_decimal():
     assert_conversion(
-        serializers.DecimalField, graphene.Float, max_digits=4, decimal_places=2
+        serializers.DecimalField, graphene.Decimal, max_digits=4, decimal_places=2
     )
 
 
diff --git a/graphene_django/rest_framework/tests/test_mutation.py b/graphene_django/rest_framework/tests/test_mutation.py
index ffbc4b5..5c2518d 100644
--- a/graphene_django/rest_framework/tests/test_mutation.py
+++ b/graphene_django/rest_framework/tests/test_mutation.py
@@ -3,7 +3,7 @@ import datetime
 from py.test import raises
 from rest_framework import serializers
 
-from graphene import Field, ResolveInfo
+from graphene import Field, ResolveInfo, NonNull, String
 from graphene.types.inputobjecttype import InputObjectType
 
 from ...types import DjangoObjectType
@@ -98,6 +98,25 @@ def test_exclude_fields():
     assert "created" not in MyMutation.Input._meta.fields
 
 
+def test_model_serializer_required_fields():
+    class MyMutation(SerializerMutation):
+        class Meta:
+            serializer_class = MyModelSerializer
+
+    assert "cool_name" in MyMutation.Input._meta.fields
+    assert MyMutation.Input._meta.fields["cool_name"].type == NonNull(String)
+
+
+def test_model_serializer_optional_fields():
+    class MyMutation(SerializerMutation):
+        class Meta:
+            serializer_class = MyModelSerializer
+            optional_fields = ("cool_name",)
+
+    assert "cool_name" in MyMutation.Input._meta.fields
+    assert MyMutation.Input._meta.fields["cool_name"].type == String
+
+
 def test_write_only_field():
     class WriteOnlyFieldModelSerializer(serializers.ModelSerializer):
         password = serializers.CharField(write_only=True)
diff --git a/graphene_django/tests/test_converter.py b/graphene_django/tests/test_converter.py
index 287ec82..df3771c 100644
--- a/graphene_django/tests/test_converter.py
+++ b/graphene_django/tests/test_converter.py
@@ -242,7 +242,7 @@ def test_should_float_convert_float():
     assert_conversion(models.FloatField, graphene.Float)
 
 
-def test_should_float_convert_decimal():
+def test_should_decimal_convert_decimal():
     assert_conversion(models.DecimalField, graphene.Decimal)
 
 
diff --git a/graphene_django/utils/testing.py b/graphene_django/utils/testing.py
index b758ac8..afe83c2 100644
--- a/graphene_django/utils/testing.py
+++ b/graphene_django/utils/testing.py
@@ -99,6 +99,10 @@ class GraphQLTestCase(TestCase):
         )
 
     @property
+    def _client(self):
+        pass
+
+    @_client.getter
     def _client(self):
         warnings.warn(
             "Using `_client` is deprecated in favour of `client`.",
@@ -107,6 +111,15 @@ class GraphQLTestCase(TestCase):
         )
         return self.client
 
+    @_client.setter
+    def _client(self, client):
+        warnings.warn(
+            "Using `_client` is deprecated in favour of `client`.",
+            PendingDeprecationWarning,
+            stacklevel=2,
+        )
+        self.client = client
+
     def assertResponseNoErrors(self, resp, msg=None):
         """
         Assert that the call went through correctly. 200 means the syntax is ok, if there are no `errors`,
diff --git a/graphene_django/utils/tests/test_testing.py b/graphene_django/utils/tests/test_testing.py
index df78321..2ef78f9 100644
--- a/graphene_django/utils/tests/test_testing.py
+++ b/graphene_django/utils/tests/test_testing.py
@@ -2,12 +2,13 @@ import pytest
 
 from .. import GraphQLTestCase
 from ...tests.test_types import with_local_registry
+from django.test import Client
 
 
 @with_local_registry
-def test_graphql_test_case_deprecated_client():
+def test_graphql_test_case_deprecated_client_getter():
     """
-    Test that `GraphQLTestCase._client`'s should raise pending deprecation warning.
+    `GraphQLTestCase._client`' getter should raise pending deprecation warning.
     """
 
     class TestClass(GraphQLTestCase):
@@ -22,3 +23,23 @@ def test_graphql_test_case_deprecated_client():
 
     with pytest.warns(PendingDeprecationWarning):
         tc._client
+
+
+@with_local_registry
+def test_graphql_test_case_deprecated_client_setter():
+    """
+    `GraphQLTestCase._client`' setter should raise pending deprecation warning.
+    """
+
+    class TestClass(GraphQLTestCase):
+        GRAPHQL_SCHEMA = True
+
+        def runTest(self):
+            pass
+
+    tc = TestClass()
+    tc._pre_setup()
+    tc.setUpClass()
+
+    with pytest.warns(PendingDeprecationWarning):
+        tc._client = Client()