strawberry-django icon indicating copy to clipboard operation
strawberry-django copied to clipboard

Allow updates of fields in nested relations

Open zvyn opened this issue 9 months ago • 5 comments

Feature Request Type

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

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

  • without the unique_together-constraint
  • with an id field on the Child
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

  • update the Child.name by setting the ID
  • add copies of Child by leaving the ID unset
  • remove a Child by omitting it

What do you think?

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

zvyn avatar Oct 04 '23 14:10 zvyn

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?

bellini666 avatar Oct 26 '23 17:10 bellini666

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!

zvyn avatar Dec 13 '23 11:12 zvyn

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

bellini666 avatar Dec 13 '23 21:12 bellini666

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

tokr-bit avatar Dec 18 '23 08:12 tokr-bit

@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 avatar Dec 22 '23 17:12 bellini666