MrThearMan / graphene-django-extensions

Extensions for writing GraphQL schemas with the graphene-django library with less boilerplate.
https://pypi.org/project/graphene-django-extensions/
MIT License
4 stars 0 forks source link

custom scalar with max length for input object type #1

Closed elyas-hedayat closed 7 months ago

elyas-hedayat commented 8 months ago

I have read the docs thoroughly before making this feature request.

I have read through other open issues, and my issue is not a duplicate.

Description

What is the current behavior?

input that can not set max_length for them:

import graphene
class BaseCreateInput(graphene.InputObjectType):
      name = graphene.String(required=True)

What is the expected behavior? input that can set max_lenght for them:

import graphene
class BaseCreateInput(graphene.InputObjectType):
      name = graphene.String(required=True,max_lenght =125)

Motivation

Main Problem That Solve with this Feature In this senario unnecessary and invalid data will not send into database.

Would you like to solve this issue yourself with a pull request?

No

MrThearMan commented 8 months ago

Hi, thanks for the feature request! This library is currently in "pre-beta" (no 0.0.1 / release in PyPI yet) but I'll keep this feature in mind for that release.

MrThearMan commented 8 months ago

I though about this feature, and I have some questions before I proceed.

The create and update mutation classes for the library use djangorestframework modelserializers, which can leverage model field's max_length automatically.

For example, given the following model:

class Example(models.Model):
    name = models.CharField(max_length=100)

...and the following serializer:

class ExampleSerializer(NestingModelSerializer):
    class Meta:
        model = Example
        fields = [
            "pk",
            "name",
        ]

We can create the following CreateMutation:

class ExampleCreateMutation(CreateMutation):
    class Meta:
        serializer_class = ExampleSerializer

Adding this to the Mutation object:

class Mutation(graphene.ObjectType):
    create_example = ExampleCreateMutation.Field()

We will get max_length validation automatically.

mutation ($input: ExampleCreateMutationInput!) {
  createExample(input:$input) {
    pk
    errors {
      field
      messages
    }
  }
}

Input data:

{
  "input": {
    "name": "<string over 100 chars>",
  }
}

Response:

{
  "data": {
    "createExample": {
      "pk": null,
      "errors": [
        {
          "field": "name",
          "messages": [
            "Ensure this field has no more than 100 characters."
          ]
        },
      ]
    }
  }
}

No need to defined GraphQL InputObjectTypes separately from the model, meaning less boilerplate and duplication of business rules.

Does this fit your usecase, or is there something I'm missing?

elyas-hedayat commented 8 months ago

that is great idea when we dealing with models for crud operation or something like this .

but think about time that we want just get email or phone number from user and send mail or SMS for them, in this senario it is significant that user enter valid e-mail address or valid PhoneNumber( for example with max 12). or when you want to get otp code that sent for user and you know that otp is fixed to 6 char but user enter more that this.

and other point : for Email is same graphene has no things named EmailScalar and if we want to get email we must use from graphen.String

MrThearMan commented 8 months ago

I see, that makes sense. For non-model operations, there exists the option for custom mutations, but as documented, those require the use of graphene fields, which don't have much validation.

I would solve this by using Regular DRF Serializers, since they have all the validation logic built-in already (e.g. max length validation and email validation, phone number validation can be achieved by django-phonenumber-field). However, there is currently some duplication required, since we'd need to declare both the graphene fields and the serializer:

from graphene_django_extensions.bases import DjangoMutation
from rest_framework import serializers

class ExampleSerializer(serializers.Serializer):
    phone = serializers.CharField(max_length=12)
    email = serializers.EmailField()

class ExampleMutation(DjangoMutation):
    class Meta:
        input_fields = {
            "phone": graphene.String(),
            "email": graphene.String(),
        }
        output_fields = {
            "successful": graphene.Boolean(),
        }

    @classmethod
    def custom_model_operation(cls, root, info, **kwargs):
        serializer = ExamplesSerializer(data=kwargs)
        serializer.is_valid(raise_exception=True)
        data = serializer.validated_data
        # 
        # Do logic with data here
        #
        return cls(successful=True)

I didn't test the above but it should be something similar.

To get rid of the duplication, I could change the DjangoMutation class to build fields from the serializer if given in Meta class:

class ExampleInputSerializer(serializers.Serializer):
    phone = serializers.CharField(max_length=12)
    email = serializers.EmailField()

class ExampleOutputSerializer(serializers.Serializer):
    successful = serializers.BoolenField()

class ExampleMutation(DjangoMutation):
    class Meta:
        serializer_class = ExampleInputSerializer
        output_serializer_class = ExampleOutputSerializer

    @classmethod
    def custom_model_operation(cls, root, info, **kwargs):
        # kwargs is already validated

How does this sound?

I should also change custom_model_operation -> custom_mutation_operation.

elyas-hedayat commented 8 months ago

that is ok overall from previous one . but here are some notes :

  1. as the link i provide here , we have already something like this (using DRF Serializer) :https://docs.graphene-python.org/projects/django/en/latest/mutations/#django-rest-framework

  2. it is also depend of DRF also , i don't think it is make a sounds good

  3. About serializer_class it will change to input? or we can customize it? this is example of DjangoModelFormMutation that we can set name for it but it is not work

                class PetMutation(DjangoModelFormMutation):
                    class Meta:
                        form_class = PetForm
                        input_field_name = 'data'
                        return_field_name = 'my_pet'
MrThearMan commented 8 months ago
  1. Yes, it seem you could achieve this by using SerializerMutation and overriding perform_mutate. In my own use, I'd like to be able to use the same mutation class for everything.

  2. For me, depending on DRF is a benefit, as there are many extensions for it that one can use, and graphene-django also already includes nice auto converting functionality for converting serializer fields to graphene fields. If you'd rather use django forms, then I'd need to think about making it an option, and how that would work.

  3. Currently, when using serializer_class for model operations, it will be used for input and output, unless one defines a output_serializer_class, which will then be used for output (for example to limit fields that can be returned back from the mutation). I was thinking of doing the same for custom mutations.

MrThearMan commented 8 months ago

Changed how custom mutations work in 0.0.5, they now require regular Serializers (not ModelSerializers). Next, I'll work on adding django forms support, as it should be pretty simple.

MrThearMan commented 8 months ago

0.0.6 now has form support for update and create, as well as custom mutations.

MrThearMan commented 7 months ago

With the support for custom mutations and form mutations added, I think this issue can be closed.