Skip to content
New issue

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

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

Already on GitHub? # to your account

Perform validation check on updating nested fields #398

Closed
1 of 3 tasks
tokr-bit opened this issue Oct 19, 2023 · 4 comments · Fixed by #659
Closed
1 of 3 tasks

Perform validation check on updating nested fields #398

tokr-bit opened this issue Oct 19, 2023 · 4 comments · 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
Contributor

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

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