strawberry-graphql / strawberry-django

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

Allow updates of fields in nested relations #383

Closed zvyn closed 5 months ago

zvyn commented 1 year ago

Feature Request Type

Description

I would like to propose supporting updates on nested relations.

Proposed solution:

  1. Look for an ID in _parse_pk and return model._default_manager.get(pk=int(value.pop("id"))), value if set
  2. Always create where #362 switched to get_or_create if the ID is optional and UNSET (but leave the behaviour unchanged for related input types without an optional ID)

Example setup copied from #360 but

from typing import Optional
import strawberry
from strawberry import auto
import strawberry_django

class Parent(models.Model):
    pass

class Child(models.Model):
    name = models.CharField(max_length=256)
    parent = models.ForeignKey(Parent, on_delete=models.CASCADE, related_name="children")

@strawberry_django.type(Child)
class ChildType:
    id: auto
    name: auto

@strawberry_django.input(Child)
class ChildInput:
    id: Optional[auto]
    name: auto

@strawberry_django.type(Parent)
class ParentType:
    id: auto
    children: list[ChildType]

@strawberry_django.input(Parent)
class ParentInput:
    children: Optional[list[ChildInput]] = strawberry_django.field()

@strawberry_django.partial(Parent)
class ParentPartial(ParentInput):
    id: auto

@strawberry.type
class Mutation:
    create_parent: ParentType = strawberry_django.mutations.create(ParentInput)
    update_parent: ParentType = strawberry_django.mutations.update(ParentPartial)

@strawberry.type
class Query:
    parents: list[ParentType] = strawberry_django.field()

schema = strawberry.Schema(
    query=Query,
    mutation=Mutation,
)

With that the client can

What do you think?

Upvote & Fund

Fund with Polar

bellini666 commented 1 year ago

Hey @zvyn ,

Is this issue fixed by https://github.com/strawberry-graphql/strawberry-graphql-django/pull/362 ? Or is there anything else that we need to do here?

zvyn commented 11 months ago

Hey @bellini666,

sorry for taking so long to respond!

This issue goes a bit further than #362:

362 enables keeping nested objects that are referenced in the mutation and already associated but does not provide a way to change properties of the nested object (instead it would fall back to the delete-and-recreate behavior).

This issue proposes a way to update properties of nested objects (provided there is a primary key to identify the nested object reliably).

If that sounds good to you, we (@geops) would be happy to implement this!

bellini666 commented 11 months ago

If that sounds good to you, we (https://github.com/geops) would be happy to implement this!

@zvyn sure, go ahead! :)

Feel free to ping me on discord if you need anything

tokr-bit commented 11 months ago

Hi @bellini666,

@zvyn and me will implement the feature.

Zvyn proposed to look for the id attribute in the data dictionary in

def _parse_pk(
    value: ParsedObject | strawberry.ID | _M | None,
    model: type[_M],
) -> tuple[_M | None, dict[str, Any] | None]:
    if value is None:
        return None, None

    if isinstance(value, Model):
        return value, None

    if isinstance(value, ParsedObject):
        return value.parse(model)

    if isinstance(value, dict):
       # do the changes here....
        return None, value

    return model._default_manager.get(pk=value), None

and in update_m2m:

  1. Look for an ID in _parse_pk and return model._default_manager.get(pk=int(value.pop("id"))), value if set
  2. Always create where https://github.com/strawberry-graphql/strawberry-graphql-django/pull/362 switched to get_or_create if the ID is optional and UNSET (but leave the behaviour unchanged for related input types without an optional ID)

However, it is possible to change the unique identifier, which is stored as self.key_attr in the DjangoCreateOrUpdateMutation class. Hardcoding to id would just cover the case of looking for objects via their id as their pk. Do you know a generic way without changing the API of the update_m2m method and its calling functions?

Thanks in advance! tokr-bit

bellini666 commented 10 months ago

@tokr-bit nice, looking forward to review the PR :)

However, it is possible to change the unique identifier, which is stored as self.key_attr in the DjangoCreateOrUpdateMutation class. Hardcoding to id would just cover the case of looking for objects via their id as their pk. Do you know a generic way without changing the API of the update_m2m method and its calling functions?

I think the easiest way is to add an extra key_attr: str | None = 'id' to it, which sould be received by the create/update functions and propagated to update_m2m and _parse_pk

Let me know if you have any other issues. Feel free to post them either here or ping me on discord

bellini666 commented 5 months ago

Fixed by https://github.com/strawberry-graphql/strawberry-django/pull/449