Skip to content

Commit

Permalink
fix: GraphQL Group filters using deleted associated objects
Browse files Browse the repository at this point in the history
There was a specific case where using Django built-in lookups for
relationships doesn't work properly with the soft delete solution we
implemented with `django-safedelete`. According to the library
maintainers, it's a known limitation[1].

To work around this limitation, a customized filter set were created for
Group schema on GraphQL. This custom filter set implements the
adjustments expected to ignore soft deleted objects when filtering by
relationships (joins).

[1] - makinacorpus/django-safedelete#158
  • Loading branch information
caiocarrara committed Jan 25, 2022
1 parent 86ff7a5 commit 701f3aa
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 13 deletions.
42 changes: 29 additions & 13 deletions terraso_backend/apps/graphql/schema/groups.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import django_filters
import graphene
from django.conf import settings
from graphene import relay
Expand All @@ -9,24 +10,40 @@
from .commons import BaseDeleteMutation, BaseWriteMutation, TerrasoConnection


class GroupNode(DjangoObjectType):
id = graphene.ID(source="pk", required=True)
class GroupFilterSet(django_filters.FilterSet):
# TODO: members__email was kept for backward compatibility. Remove as soon
# as the web client be updated to use memberships__email filter
members__email = django_filters.CharFilter(method="filter_memberships_email")
memberships__email = django_filters.CharFilter(method="filter_memberships_email")
associated_landscapes__is_default_landscape_group = django_filters.BooleanFilter(
method="filter_associated_landscapes"
)
associated_landscapes__isnull = django_filters.BooleanFilter(
method="filter_associated_landscapes"
)

class Meta:
model = Group
filter_fields = {
fields = {
"name": ["exact", "icontains", "istartswith"],
"slug": ["exact", "icontains"],
"description": ["icontains"],
"associations_as_parent__child_group": ["exact"],
"associations_as_child__parent_group": ["exact"],
"associations_as_parent__child_group__slug": ["icontains"],
"associations_as_child__parent_group__slug": ["icontains"],
"memberships": ["exact"],
"associated_landscapes__is_default_landscape_group": ["exact"],
"associated_landscapes": ["isnull"],
"members__email": ["exact"],
}

def filter_memberships_email(self, queryset, name, value):
return queryset.filter(memberships__user__email=value, memberships__deleted_at__isnull=True)

def filter_associated_landscapes(self, queryset, name, value):
filters = {"associated_landscapes__deleted_at__isnull": True}
filters[name] = value
return queryset.filter(**filters)


class GroupNode(DjangoObjectType):
id = graphene.ID(source="pk", required=True)

class Meta:
model = Group
fields = (
"name",
"slug",
Expand All @@ -39,6 +56,7 @@ class Meta:
"associations_as_child",
"associated_landscapes",
)
filterset_class = GroupFilterSet
interfaces = (relay.Node,)
connection_class = TerrasoConnection

Expand Down Expand Up @@ -102,8 +120,6 @@ def mutate_and_get_payload(cls, root, info, **kwargs):

ff_check_permission_on = settings.FEATURE_FLAGS["CHECK_PERMISSIONS"]
user_has_delete_permission = user.has_perm(Group.get_perm("delete"), obj=kwargs["id"])

if ff_check_permission_on and not user_has_delete_permission:
raise GraphQLValidationException("User has no permission to delete this data.")

return super().mutate_and_get_payload(root, info, **kwargs)
141 changes: 141 additions & 0 deletions terraso_backend/tests/graphql/test_groups_filters.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import pytest
from mixer.backend.django import mixer

from apps.core.models import Group, Membership

pytestmark = pytest.mark.django_db


def test_groups_filter_by_membership_user_ignores_deleted_memberships(client_query, memberships):
membership = memberships[0]
membership.delete()

response = client_query(
"""
{groups(memberships_Email: "%s") {
edges {
node {
slug
}
}
}}
"""
% membership.user.email
)
edges = response.json()["data"]["groups"]["edges"]

assert not edges


def test_groups_filter_by_with_landscape_association(client_query, users, landscape_groups):
user = users[0]
default_group_association, common_group_association = landscape_groups
mixer.blend(Membership, user=user, group=default_group_association.group)
mixer.blend(Membership, user=user, group=common_group_association.group)

response = client_query(
"""
{groups(
associatedLandscapes_Isnull: false,
memberships_Email: "%s"
) {
edges {
node {
slug
}
}
}}
"""
% user.email
)
edges = response.json()["data"]["groups"]["edges"]
groups_result = [edge["node"]["slug"] for edge in edges]

assert len(groups_result) == 2
assert default_group_association.group.slug in groups_result
assert common_group_association.group.slug in groups_result


def test_groups_filter_by_with_landscape_association_ignores_deleted_associations(
client_query, users, landscape_groups
):
user = users[0]
default_group_association, common_group_association = landscape_groups
mixer.blend(Membership, user=user, group=default_group_association.group)
mixer.blend(Membership, user=user, group=common_group_association.group)

default_group_association.delete()

response = client_query(
"""
{groups(
associatedLandscapes_Isnull: false,
memberships_Email: "%s"
) {
edges {
node {
slug
}
}
}}
"""
% user.email
)
edges = response.json()["data"]["groups"]["edges"]
groups_result = [edge["node"]["slug"] for edge in edges]

assert len(groups_result) == 1


def test_groups_filter_by_default_landscape_group(client_query, users, landscape_groups):
user = users[0]
default_group_association, common_group_association = landscape_groups
mixer.blend(Membership, user=user, group=default_group_association.group)
mixer.blend(Membership, user=user, group=common_group_association.group)

response = client_query(
"""
{groups(
associatedLandscapes_IsDefaultLandscapeGroup: true,
memberships_Email: "%s"
) {
edges {
node {
slug
}
}
}}
"""
% user.email
)
edges = response.json()["data"]["groups"]["edges"]
groups_result = [edge["node"]["slug"] for edge in edges]

assert len(groups_result) == 1
assert default_group_association.group.slug in groups_result


def test_groups_filter_by_without_landscape_association(client_query, users):
user = users[0]
group = mixer.blend(Group)
membership = mixer.blend(Membership, user=user, group=group)

response = client_query(
"""
{groups(
associatedLandscapes_Isnull: true,
memberships_Email: "%s"
) {
edges {
node {
slug
}
}
}}
"""
% membership.user.email
)
edges = response.json()["data"]["groups"]["edges"]
groups_result = [edge["node"]["slug"] for edge in edges]
assert len(groups_result) == 1
assert groups_result[0] == group.slug

0 comments on commit 701f3aa

Please sign in to comment.