Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perform validation check on updating nested fields #398

Open
1 of 3 tasks
tokr-bit opened this issue Oct 19, 2023 · 4 comments · May be fixed by #659
Open
1 of 3 tasks

Perform validation check on updating nested fields #398

tokr-bit opened this issue Oct 19, 2023 · 4 comments · May be fixed by #659
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@tokr-bit
Copy link
Contributor

tokr-bit commented Oct 19, 2023

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

  • Core functionality
  • Alteration (enhancement/optimization) of existing feature(s)
  • New behavior

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

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@bellini666
Copy link
Member

Hey @tokr-bit ,

Thanks for reporting this!

I'm wondering if #394 will also fix this issue?

cc @sdobbelaere

@tokr-bit
Copy link
Contributor Author

tokr-bit commented Oct 30, 2023

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
Copy link
Contributor

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

@philipstarkey
Copy link

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"
            ]
        }
    ],
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
4 participants