Passing concrete instances to ListInput[NodeInput] when using model inheritance causes unintended removal of objects #793
Description
Describe the Bug
Given the following polymorphic Django models using django-model-util's InheritanceManager -
from __future__ import annotations
from django.db.models import ManyToManyField
from django_stubs_ext.db.models import TypedModelMeta
from model_utils.managers import InheritanceManager
class Project(Model):
"""An abstract project."""
# ...
dependencies = ManyToManyField(
"self",
symmetrical=False,
related_name="dependants",
help_text="A list of projects required by this project.",
blank=True,
)
# ...
objects = InheritanceManager()
class Meta(TypedModelMeta):
verbose_name = "project"
verbose_name_plural = "projects"
class ExternalProject(Project):
"""An external project."""
class Meta:
verbose_name = "external project"
verbose_name_plural = "external projects"And a Strawberry GraphQL type that allows mutating the dependencies and dependants many-to-many relationship -
#...
@strawberry_django.partial(models.Project, description="A partial project input.")
class ProjectInputPartial(NodeInput):
"""A partial project input."""
# ...
dependencies: ListInput[NodeInput] | None = strawberry_django.field(
description="An updated list of projects required by this project."
)
dependants: ListInput[NodeInput] | None = strawberry_django.field(
description="An updated list of projects that require this project."
)
# ...A used in a Strawberry Django CUD mutation like so -
class ProjectUpdateMutation(DjangoUpdateMutation):
"""A mutation for updating a project."""
def __init__(self, name: str | None = None, **kwargs: Any) -> None:
super().__init__(graphql_name=name, **kwargs)
@property
def django_model(self) -> type[Model]:
return super().django_model or models.Project
# ...
create_external_project: Any = partial(ProjectCreateMutation, input_type=ExternalProjectInput)
update_external_project: Any = partial(ProjectUpdateMutation, input_type=ExternalProjectInputPartial)
# ...
update_project: Any = partial(ProjectUpdateMutation, input_type=ProjectInputPartial)
# ...And using it in your Strawberry schema like so -
@strawberry.type
class ProjectsMutation:
"""A collection of project-related GraphQL mutations."""
update_external_project: ExternalProject = mutations.update_external_project(
description="Updates an external project.",
extensions=[HasPerm("projects.change_project", fail_silently=False), HasScope("write:project")],
)
# ...
# NB: The below mutations act on the `Project` interface. Since the mutations already return a union
# with `OperationInfo`, and interfaces are prohibited in unions, we must expand the full `Project` type.
update_project: ExternalProject | ... | WebProject = mutations.update_project(
description="Updates a generic project.",
extensions=[HasPerm("projects.change_project", fail_silently=False), HasScope("write:project")],
)
# ...And executing via the following query -
mutation UpdateProject {
updateProject(data: {
id: "RXh0ZXJuYWxQcm9qZWN0Ojk="
dependencies: { set: [
{ id: "V2ViUHJvamVjdDox" },
{ id: "V2ViUHJvamVjdDoy" }
] }
dependants: { set: [
{ id: "RXh0ZXJuYWxQcm9qZWN0OjE=" }
] }
}) {
__typename
... on Project {
id
slug
}
... on OperationInfo {
messages {
code
field
message
}
}
}
}It will reach the update_m2m function which fetches the existing objects in the relationship:
This yields a set of abstract models existing: set[Project].
Since the mutation was a ListInput[NodeInput] (i.e. we pass { id: "RXh0ZXJuYWxQcm9qZWN0OjE=" }) this yields obj: ExternalProject.
Later, it will check if obj not in existing, which is always true because it is trying to compare the concrete model to the abstract model.
strawberry-django/strawberry_django/mutations/resolvers.py
Lines 697 to 700 in 269954c
Then, it tries to existing.discard(obj) which does nothing, because obj: ExternalProject is not in set[Project].
Which means it gets to the end, re-adds the objects to the relationship, but then existing has all the original projects in it, and so it for remaining in existing removes them from the relationship immediately after.
Solution
A possible solution would be to upcast concrete types (e.g. ExternalProject) into their abstract type (e.g. Project) in values before checking if obj not in existing.
Another solution is to change the line existing = set(manager.all()) to account for inheritance querysets.
existing = set(manager.select_subclasses() if isinstance(manager, InheritanceManager) else manager.all())Workaround
Ensure the passed id is for the abstract Project node and not the concrete type. This is not feasible and abstract Project IDs are never returned to the client as they only ever query concrete types.
Is there a way to avoid the complexities of NodeInput and just pass dependencies: { set: [ "slug-1", "slug-2" ] } and therefore avoid concrete types altogether?
System Information
- Operating system: Windows
- Python version:
3.11 - Strawberry version (if applicable):
0.65.1