strawberry-graphql / strawberry-django

Strawberry GraphQL Django extension
https://strawberry.rocks/docs/django
MIT License
419 stars 122 forks source link

Perform validation check on updating nested fields #398

Open tokr-bit opened 1 year ago

tokr-bit commented 1 year ago

Summary

If a nested field is updated via the parent field, a validation on the nested field should be performed, if some validators are provided on the django field.

Feature Request Type

Description

Assuming we have a Parent model with some nested fields:

class Parent(models.Model);
   foo_field = models.CharField()

class Child(models.Model):
   url = models.URLField(validators=[validators.validate_url])
   parent_field = models.ForeignKey(to=Parent, related_name="children")

We create some types and a create mutation:

@strawberry.type(Child, exclude=["parent_field"])
class Child:
    id: auto
    url: auto

@strawberry.input(Child)
class ChildInput(Child):
   pass

@strawberry.type(Parent)
class Parent:
   id: auto
   foo_field: auto
   children: List[Child]

@strawberry.input(Parent)
class ParentInput(Parent):
   pass

@strawberry.type
class Mutation:
    parent: Parent = mutations.create(types.ParentInput)

No matter which value is passed to the URL Field of the Child, the validator is not executed.

Solution

Reason is that the update_m2m method in resolvers.py does not execute a full_clean on the respective object.

Upvote & Fund

Fund with Polar

bellini666 commented 1 year ago

Hey @tokr-bit ,

Thanks for reporting this!

I'm wondering if https://github.com/strawberry-graphql/strawberry-graphql-django/pull/394 will also fix this issue?

cc @sdobbelaere

tokr-bit commented 1 year ago

Hey,

394 won't fix the issue since the update_m2m method is not changed. If you and @sdobbelaere support the feature request, I can supply a PR.

sdobbelaere commented 1 year ago

@tokr-bit I would appreciate this fix. I feel this would be something I'd run into myself as well during my project.

philipstarkey commented 1 week ago

I'm not sure if this is why the issue was left open (since #405 was meant to fix it?) but there is an additional issue even with the fix where integrity errors raised by the creation happen before the call to full_clean(), which can lead to messages like this that leak model constraint and field names

{
    "data": null,
    "errors": [
        {
            "message": "duplicate key value violates unique constraint \"project_tags_unique_label_per_issue\"\nDETAIL:  Key (label, issue_id)=(L1, 55) already exists.\n",
            "locations": [
                {
                    "line": 2,
                    "column": 3
                }
            ],
            "path": [
                "createIssueWithTags"
            ]
        }
    ],
}