From b961c3675f5b986bb3712da58effca36039bfec5 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Thu, 5 Oct 2023 15:59:28 -0500 Subject: [PATCH 01/34] refactor: Added membership_list to landscape --- .../core/landscape_collaboration_roles.py | 18 +++ .../apps/core/models/landscapes.py | 6 + .../apps/graphql/schema/landscapes.py | 1 + .../apps/graphql/schema/schema.graphql | 111 +++++++++--------- terraso_backend/tests/graphql/conftest.py | 21 ++++ .../tests/graphql/test_landscape.py | 33 ++++++ todo.md | 1 + 7 files changed, 136 insertions(+), 55 deletions(-) create mode 100644 terraso_backend/apps/core/landscape_collaboration_roles.py create mode 100644 todo.md diff --git a/terraso_backend/apps/core/landscape_collaboration_roles.py b/terraso_backend/apps/core/landscape_collaboration_roles.py new file mode 100644 index 000000000..e677e6b64 --- /dev/null +++ b/terraso_backend/apps/core/landscape_collaboration_roles.py @@ -0,0 +1,18 @@ +# Copyright © 2023 Technology Matters +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published +# by the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see https://www.gnu.org/licenses/. + + +ROLE_MANAGER = "manager" +ROLE_MEMBER = "member" diff --git a/terraso_backend/apps/core/models/landscapes.py b/terraso_backend/apps/core/models/landscapes.py index 1cdae393c..de329b6ca 100644 --- a/terraso_backend/apps/core/models/landscapes.py +++ b/terraso_backend/apps/core/models/landscapes.py @@ -64,6 +64,12 @@ class Landscape(SlugModel, DirtyFieldsMixin): related_name="created_landscapes", ) groups = models.ManyToManyField(Group, through="LandscapeGroup") + membership_list = models.ForeignKey( + "collaboration.MembershipList", + on_delete=models.CASCADE, + related_name="landscape", + null=True, + ) area_types = models.JSONField(blank=True, null=True) taxonomy_terms = models.ManyToManyField(TaxonomyTerm, blank=True) diff --git a/terraso_backend/apps/graphql/schema/landscapes.py b/terraso_backend/apps/graphql/schema/landscapes.py index 260553e9a..632d5588a 100644 --- a/terraso_backend/apps/graphql/schema/landscapes.py +++ b/terraso_backend/apps/graphql/schema/landscapes.py @@ -72,6 +72,7 @@ class Meta: "profile_image", "profile_image_description", "center_coordinates", + "membership_list", ) interfaces = (relay.Node,) diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index 5402a63f8..c7a8dadfe 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -286,6 +286,7 @@ type LandscapeNode implements Node { email: String! areaScalarM2: Float createdBy: UserNode + membershipList: CollaborationMembershipListNode taxonomyTerms(offset: Int, before: String, after: String, first: Int, last: Int, type: CoreTaxonomyTermTypeChoices, type_In: [CoreTaxonomyTermTypeChoices]): TaxonomyTermNodeConnection! population: Int partnershipStatus: CoreLandscapePartnershipStatusChoices @@ -309,6 +310,61 @@ schema (one of the key benefits of GraphQL). """ scalar JSONString +type CollaborationMembershipListNode implements Node { + membershipType: CollaborationMembershipListMembershipTypeChoices! + memberships(offset: Int, before: String, after: String, first: Int, last: Int, user: ID, user_In: [ID], userRole: String, user_Email_Icontains: String, user_Email_In: [String], membershipStatus: CollaborationMembershipMembershipStatusChoices, user_Email_Not: String): CollaborationMembershipNodeConnection! + id: ID! + accountMembership: CollaborationMembershipNode + membershipsCount: Int +} + +"""An enumeration.""" +enum CollaborationMembershipListMembershipTypeChoices { + """Open""" + OPEN + + """Closed""" + CLOSED +} + +type CollaborationMembershipNodeConnection { + """Pagination data for this connection.""" + pageInfo: PageInfo! + + """Contains the nodes in this connection.""" + edges: [CollaborationMembershipNodeEdge!]! + totalCount: Int! +} + +""" +A Relay edge containing a `CollaborationMembershipNode` and its cursor. +""" +type CollaborationMembershipNodeEdge { + """The item at the end of the edge""" + node: CollaborationMembershipNode! + + """A cursor for use in pagination""" + cursor: String! +} + +type CollaborationMembershipNode implements Node { + membershipList: CollaborationMembershipListNode! + user: UserNode + userRole: String! + membershipStatus: CollaborationMembershipMembershipStatusChoices! + pendingEmail: String + id: ID! +} + +"""An enumeration.""" +enum CollaborationMembershipMembershipStatusChoices { + """Approved""" + APPROVED + + """Pending""" + PENDING +} + type TaxonomyTermNodeConnection { """Pagination data for this connection.""" pageInfo: PageInfo! @@ -598,61 +654,6 @@ type StoryMapNode implements Node { membershipList: CollaborationMembershipListNode } -type CollaborationMembershipListNode implements Node { - membershipType: CollaborationMembershipListMembershipTypeChoices! - memberships(offset: Int, before: String, after: String, first: Int, last: Int, user: ID, user_In: [ID], userRole: String, user_Email_Icontains: String, user_Email_In: [String], membershipStatus: CollaborationMembershipMembershipStatusChoices, user_Email_Not: String): CollaborationMembershipNodeConnection! - id: ID! - accountMembership: CollaborationMembershipNode - membershipsCount: Int -} - -"""An enumeration.""" -enum CollaborationMembershipListMembershipTypeChoices { - """Open""" - OPEN - - """Closed""" - CLOSED -} - -type CollaborationMembershipNodeConnection { - """Pagination data for this connection.""" - pageInfo: PageInfo! - - """Contains the nodes in this connection.""" - edges: [CollaborationMembershipNodeEdge!]! - totalCount: Int! -} - -""" -A Relay edge containing a `CollaborationMembershipNode` and its cursor. -""" -type CollaborationMembershipNodeEdge { - """The item at the end of the edge""" - node: CollaborationMembershipNode! - - """A cursor for use in pagination""" - cursor: String! -} - -type CollaborationMembershipNode implements Node { - membershipList: CollaborationMembershipListNode! - user: UserNode - userRole: String! - membershipStatus: CollaborationMembershipMembershipStatusChoices! - pendingEmail: String - id: ID! -} - -"""An enumeration.""" -enum CollaborationMembershipMembershipStatusChoices { - """Approved""" - APPROVED - - """Pending""" - PENDING -} - type StoryMapNodeConnection { """Pagination data for this connection.""" pageInfo: PageInfo! diff --git a/terraso_backend/tests/graphql/conftest.py b/terraso_backend/tests/graphql/conftest.py index 02478a8a5..10480ac5a 100644 --- a/terraso_backend/tests/graphql/conftest.py +++ b/terraso_backend/tests/graphql/conftest.py @@ -24,6 +24,7 @@ from apps.auth.services import JWTService from apps.collaboration.models import Membership as CollaborationMembership from apps.collaboration.models import MembershipList +from apps.core import landscape_collaboration_roles from apps.core.models import ( Group, GroupAssociation, @@ -108,6 +109,26 @@ def landscapes(): ) +@pytest.fixture +def landscape_membership_list(landscapes): + landscape = landscapes[0] + landscape.membership_list = mixer.blend(MembershipList) + landscape.save() + + return landscape.membership_list + + +@pytest.fixture +def landscape_user_memberships(landscape_membership_list, users): + return mixer.cycle(2).blend( + CollaborationMembership, + membership_list=landscape_membership_list, + user=(u for u in users), + user_role=landscape_collaboration_roles.ROLE_MEMBER, + pending_email=None, + ) + + @pytest.fixture def managed_landscapes(users): landscapes = mixer.cycle(2).blend(Landscape) diff --git a/terraso_backend/tests/graphql/test_landscape.py b/terraso_backend/tests/graphql/test_landscape.py index 8881b00d2..c1ec1f3d7 100644 --- a/terraso_backend/tests/graphql/test_landscape.py +++ b/terraso_backend/tests/graphql/test_landscape.py @@ -93,3 +93,36 @@ def test_landscapes_query_with_json_polygon(client_query, landscapes): for landscape in landscapes: assert landscape.area_polygon in landscapes_result + + +def test_landscapes_query_with_membership(client_query, landscapes, landscape_user_memberships): + response = client_query( + """ + {landscapes(slug: "%s") { + edges { + node { + name + membershipList { + memberships { + edges { + node { + user { + email + } + } + } + } + } + } + } + }} + """ + % landscapes[0].slug + ) + + json_response = response.json() + + memberships = json_response["data"]["landscapes"]["edges"][0]["node"]["membershipList"][ + "memberships" + ]["edges"] + assert len(memberships) == 2 diff --git a/todo.md b/todo.md new file mode 100644 index 000000000..0ecb5e25d --- /dev/null +++ b/todo.md @@ -0,0 +1 @@ +- Check that only members can see landscape membership list \ No newline at end of file From 613e4d45fd9e51bbd37fe184ae798a799a31f7e6 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Fri, 6 Oct 2023 10:49:04 -0500 Subject: [PATCH 02/34] refactor: Create manager when landscape created --- .../apps/core/models/landscapes.py | 28 +++++++++++-------- .../apps/graphql/schema/schema.graphql | 2 +- .../mutations/test_landscape_mutations.py | 10 ++++++- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/terraso_backend/apps/core/models/landscapes.py b/terraso_backend/apps/core/models/landscapes.py index de329b6ca..4373878c5 100644 --- a/terraso_backend/apps/core/models/landscapes.py +++ b/terraso_backend/apps/core/models/landscapes.py @@ -16,6 +16,7 @@ import structlog from dirtyfields import DirtyFieldsMixin from django.contrib.contenttypes.fields import GenericRelation +from django.apps import apps from django.db import models, transaction from apps.core import permission_rules as perm_rules @@ -25,6 +26,7 @@ ) from apps.core.models.taxonomy_terms import TaxonomyTerm +from ..landscape_collaboration_roles import ROLE_MANAGER from .commons import BaseModel, SlugModel, validate_name from .groups import Group from .shared_resources import SharedResource @@ -68,7 +70,6 @@ class Landscape(SlugModel, DirtyFieldsMixin): "collaboration.MembershipList", on_delete=models.CASCADE, related_name="landscape", - null=True, ) area_types = models.JSONField(blank=True, null=True) @@ -107,6 +108,9 @@ class Meta(SlugModel.Meta): _unique_fields = ["name"] abstract = False + def full_clean(self, *args, **kwargs): + super().full_clean(*args, **kwargs, exclude=["membership_list"]) + def save(self, *args, **kwargs): dirty_fields = self.get_dirty_fields() if self.area_polygon and "area_polygon" in dirty_fields: @@ -116,21 +120,23 @@ def save(self, *args, **kwargs): self.center_coordinates = calculate_geojson_centroid(self.area_polygon) with transaction.atomic(): + MembershipList = apps.get_model("collaboration", "MembershipList") + Membership = apps.get_model("collaboration", "Membership") creating = not Landscape.objects.filter(pk=self.pk).exists() - super().save(*args, **kwargs) - if creating and self.created_by: - group = Group( - name="Group {}".format(self.slug), - description="", - created_by=self.created_by, + self.membership_list = MembershipList.objects.create( + enroll_method=MembershipList.ENROLL_METHOD_BOTH, + membership_type=MembershipList.MEMBERSHIP_TYPE_OPEN, ) - group.save() - landscape_group = LandscapeGroup( - group=group, landscape=self, is_default_landscape_group=True + membership = Membership( + membership_list=self.membership_list, + user=self.created_by, + user_role=ROLE_MANAGER, ) - landscape_group.save() + membership.save() + + super().save(*args, **kwargs) def delete(self, *args, **kwargs): default_group = self.get_default_group() diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index c7a8dadfe..88860f5ae 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -286,7 +286,7 @@ type LandscapeNode implements Node { email: String! areaScalarM2: Float createdBy: UserNode - membershipList: CollaborationMembershipListNode + membershipList: CollaborationMembershipListNode! taxonomyTerms(offset: Int, before: String, after: String, first: Int, last: Int, type: CoreTaxonomyTermTypeChoices, type_In: [CoreTaxonomyTermTypeChoices]): TaxonomyTermNodeConnection! population: Int partnershipStatus: CoreLandscapePartnershipStatusChoices diff --git a/terraso_backend/tests/graphql/mutations/test_landscape_mutations.py b/terraso_backend/tests/graphql/mutations/test_landscape_mutations.py index c1ceac62b..e5e1cc0f8 100644 --- a/terraso_backend/tests/graphql/mutations/test_landscape_mutations.py +++ b/terraso_backend/tests/graphql/mutations/test_landscape_mutations.py @@ -37,11 +37,19 @@ def test_landscapes_add(client_query): """, variables={"input": {"name": landscape_name}}, ) - landscape_result = response.json()["data"]["addLandscape"]["landscape"] + json_response = response.json() + + landscape_result = json_response["data"]["addLandscape"]["landscape"] assert landscape_result["id"] assert landscape_result["name"] == landscape_name + # Assert user is added as manager + landscape = Landscape.objects.get(id=landscape_result["id"]) + manager_membership = landscape.membership_list.memberships.first() + assert manager_membership.user_role == "manager" + assert manager_membership.user == landscape.created_by + def test_landscapes_add_has_created_by_filled_out(client_query, users): landscape_name = "Testing Landscape" From 4033bc7aaec12cd099bb89c337cb303027525de1 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Fri, 6 Oct 2023 10:55:46 -0500 Subject: [PATCH 03/34] fix: Fixed manager test --- terraso_backend/tests/core/models/test_landscapes.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/terraso_backend/tests/core/models/test_landscapes.py b/terraso_backend/tests/core/models/test_landscapes.py index 34b2120ac..952d67616 100644 --- a/terraso_backend/tests/core/models/test_landscapes.py +++ b/terraso_backend/tests/core/models/test_landscapes.py @@ -20,7 +20,8 @@ from django.core.exceptions import ValidationError from mixer.backend.django import mixer -from apps.core.models.groups import Group, Membership +from apps.core import landscape_collaboration_roles +from apps.core.models.groups import Group from apps.core.models.landscapes import Landscape, LandscapeGroup from apps.core.models.users import User @@ -116,10 +117,9 @@ def test_landscape_get_default_group_without_group_returns_none(): def test_landscape_creator_becomes_manager(): user = mixer.blend(User) landscape = mixer.blend(Landscape, created_by=user) + manager_membership = landscape.membership_list.memberships.get(user=user) - manager_membership = Membership.objects.get(group=landscape.get_default_group(), user=user) - - assert manager_membership.user_role == Membership.ROLE_MANAGER + assert manager_membership.user_role == landscape_collaboration_roles.ROLE_MANAGER def test_landscape_area_calculated(unit_polygon): From 1e81edbd3a92b2b6932498ec5911a3be0fe9fd6d Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Fri, 6 Oct 2023 11:28:32 -0500 Subject: [PATCH 04/34] refactor: User landscape manager --- .../apps/collaboration/models/memberships.py | 10 +++++++- .../apps/core/models/landscapes.py | 7 ++---- terraso_backend/apps/core/models/users.py | 7 +++--- .../tests/core/models/test_users.py | 24 ++++++------------- terraso_backend/tests/graphql/conftest.py | 11 ++++----- 5 files changed, 26 insertions(+), 33 deletions(-) diff --git a/terraso_backend/apps/collaboration/models/memberships.py b/terraso_backend/apps/collaboration/models/memberships.py index d4500e52d..8e32ac903 100644 --- a/terraso_backend/apps/collaboration/models/memberships.py +++ b/terraso_backend/apps/collaboration/models/memberships.py @@ -54,8 +54,16 @@ class MembershipList(BaseModel): default=DEFAULT_MEMERBSHIP_TYPE, ) + def default_validation_func(self): + return False + def save_membership( - self, user_email, user_role, membership_status, validation_func, membership_class=None + self, + user_email, + user_role, + membership_status, + validation_func=default_validation_func, + membership_class=None, ): membership_class = membership_class or Membership user = User.objects.filter(email__iexact=user_email).first() diff --git a/terraso_backend/apps/core/models/landscapes.py b/terraso_backend/apps/core/models/landscapes.py index 4373878c5..00493acfb 100644 --- a/terraso_backend/apps/core/models/landscapes.py +++ b/terraso_backend/apps/core/models/landscapes.py @@ -129,12 +129,9 @@ def save(self, *args, **kwargs): enroll_method=MembershipList.ENROLL_METHOD_BOTH, membership_type=MembershipList.MEMBERSHIP_TYPE_OPEN, ) - membership = Membership( - membership_list=self.membership_list, - user=self.created_by, - user_role=ROLE_MANAGER, + self.membership_list.save_membership( + self.created_by.email, ROLE_MANAGER, Membership.APPROVED ) - membership.save() super().save(*args, **kwargs) diff --git a/terraso_backend/apps/core/models/users.py b/terraso_backend/apps/core/models/users.py index bb805175b..b782467b6 100644 --- a/terraso_backend/apps/core/models/users.py +++ b/terraso_backend/apps/core/models/users.py @@ -21,6 +21,8 @@ from django.utils.translation import gettext_lazy as _ from safedelete.models import SOFT_DELETE_CASCADE, SafeDeleteManager, SafeDeleteModel +from .. import landscape_collaboration_roles + NOTIFICATION_KEY_GROUP = "group_notifications" NOTIFICATION_KEY_STORY_MAP = "story_map_notifications" NOTIFICATION_KEY_LANGUAGE = "language" @@ -99,10 +101,9 @@ def save(self, *args, **kwargs): def is_landscape_manager(self, landscape_id): return ( - self.memberships.managers_only() + self.collaboration_memberships.by_role(landscape_collaboration_roles.ROLE_MANAGER) .filter( - group__associated_landscapes__is_default_landscape_group=True, - group__associated_landscapes__landscape__pk=landscape_id, + membership_list__landscape__pk=landscape_id, ) .exists() ) diff --git a/terraso_backend/tests/core/models/test_users.py b/terraso_backend/tests/core/models/test_users.py index e0cc92313..83190b5b0 100644 --- a/terraso_backend/tests/core/models/test_users.py +++ b/terraso_backend/tests/core/models/test_users.py @@ -16,7 +16,9 @@ import pytest from mixer.backend.django import mixer -from apps.core.models import Group, Landscape, LandscapeGroup, User +from apps.collaboration.models import Membership as CollaborationMembership +from apps.core import landscape_collaboration_roles +from apps.core.models import Landscape, User pytestmark = pytest.mark.django_db @@ -37,23 +39,11 @@ def test_user_string_remove_spaces_from_name(): assert user_last_name.strip() == user.last_name -@pytest.mark.parametrize( - "is_default_landscape_group, is_expected_to_be_manager", - ( - (True, True), - (False, False), - ), -) -def test_user_is_landscape_manager(is_default_landscape_group, is_expected_to_be_manager): +def test_user_is_landscape_manager(): user = mixer.blend(User) - group = mixer.blend(Group) landscape = mixer.blend(Landscape) - mixer.blend( - LandscapeGroup, - landscape=landscape, - group=group, - is_default_landscape_group=is_default_landscape_group, + landscape.membership_list.save_membership( + user.email, landscape_collaboration_roles.ROLE_MANAGER, CollaborationMembership.APPROVED ) - group.add_manager(user) - assert user.is_landscape_manager(landscape.id) is is_expected_to_be_manager + assert user.is_landscape_manager(landscape.id) is True diff --git a/terraso_backend/tests/graphql/conftest.py b/terraso_backend/tests/graphql/conftest.py index 10480ac5a..1568f93f6 100644 --- a/terraso_backend/tests/graphql/conftest.py +++ b/terraso_backend/tests/graphql/conftest.py @@ -134,13 +134,10 @@ def managed_landscapes(users): landscapes = mixer.cycle(2).blend(Landscape) for i in range(len(landscapes)): - group = mixer.blend(Group) - group.add_manager(users[i]) - mixer.blend( - LandscapeGroup, - landscape=landscapes[i], - group=group, - is_default_landscape_group=True, + landscapes[i].membership_list.save_membership( + users[i].email, + landscape_collaboration_roles.ROLE_MANAGER, + CollaborationMembership.APPROVED, ) return landscapes From 412ec33e972048291ba33b05f7b57f30b2f015f9 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Fri, 6 Oct 2023 12:06:02 -0500 Subject: [PATCH 05/34] refactor: Removed default landscape group from tests and GQL --- .../apps/graphql/schema/landscape_groups.py | 2 - .../apps/graphql/schema/schema.graphql | 7 ++- terraso_backend/tests/graphql/conftest.py | 13 ++---- .../test_landscape_groups_mutations.py | 29 ++++++------ .../mutations/test_landscape_mutations.py | 5 +- .../tests/graphql/test_groups_filters.py | 46 +++---------------- .../tests/graphql/test_landscape_groups.py | 16 +++---- terraso_backend/tests/storage/conftest.py | 12 +++-- terraso_backend/tests/storage/test_views.py | 4 +- 9 files changed, 50 insertions(+), 84 deletions(-) diff --git a/terraso_backend/apps/graphql/schema/landscape_groups.py b/terraso_backend/apps/graphql/schema/landscape_groups.py index 444707c01..2d10d9fd6 100644 --- a/terraso_backend/apps/graphql/schema/landscape_groups.py +++ b/terraso_backend/apps/graphql/schema/landscape_groups.py @@ -42,13 +42,11 @@ class Meta: "landscape__slug": ["icontains"], "group": ["exact"], "group__slug": ["icontains"], - "is_default_landscape_group": ["exact"], "is_partnership": ["exact"], } fields = ( "landscape", "group", - "is_default_landscape_group", "is_partnership", "partnership_year", ) diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index 88860f5ae..9e78910d2 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -31,7 +31,7 @@ type Query { groups(offset: Int, before: String, after: String, first: Int, last: Int, name: String, name_Icontains: String, name_Istartswith: String, slug: String, slug_Icontains: String, description_Icontains: String, memberships_Email: String, associatedLandscapes_IsDefaultLandscapeGroup: Boolean, associatedLandscapes_Isnull: Boolean, associatedLandscapes_IsPartnership: Boolean): GroupNodeConnection landscapes(offset: Int, before: String, after: String, first: Int, last: Int, name_Icontains: String, description_Icontains: String, slug: String, slug_Icontains: String, website_Icontains: String, location_Icontains: String): LandscapeNodeConnection users(offset: Int, before: String, after: String, first: Int, last: Int, email: String, email_Icontains: String, email_Iexact: String, firstName_Icontains: String, lastName_Icontains: String, project: String): UserNodeConnection - landscapeGroups(offset: Int, before: String, after: String, first: Int, last: Int, landscape: ID, landscape_Slug_Icontains: String, group: ID, group_Slug_Icontains: String, isDefaultLandscapeGroup: Boolean, isPartnership: Boolean): LandscapeGroupNodeConnection + landscapeGroups(offset: Int, before: String, after: String, first: Int, last: Int, landscape: ID, landscape_Slug_Icontains: String, group: ID, group_Slug_Icontains: String, isPartnership: Boolean): LandscapeGroupNodeConnection memberships(offset: Int, before: String, after: String, first: Int, last: Int, group: ID, group_In: [ID], group_Slug_Icontains: String, group_Slug_In: [String], user: ID, user_In: [ID], userRole: CoreMembershipUserRoleChoices, user_Email_Icontains: String, user_Email_In: [String], membershipStatus: CoreMembershipMembershipStatusChoices): MembershipNodeConnection groupAssociations(offset: Int, before: String, after: String, first: Int, last: Int, parentGroup: ID, childGroup: ID, parentGroup_Slug_Icontains: String, childGroup_Slug_Icontains: String): GroupAssociationNodeConnection dataEntry( @@ -107,7 +107,7 @@ type GroupNode implements Node { associationsAsParent(offset: Int, before: String, after: String, first: Int, last: Int, parentGroup: ID, childGroup: ID, parentGroup_Slug_Icontains: String, childGroup_Slug_Icontains: String): GroupAssociationNodeConnection! associationsAsChild(offset: Int, before: String, after: String, first: Int, last: Int, parentGroup: ID, childGroup: ID, parentGroup_Slug_Icontains: String, childGroup_Slug_Icontains: String): GroupAssociationNodeConnection! memberships(offset: Int, before: String, after: String, first: Int, last: Int, group: ID, group_In: [ID], group_Slug_Icontains: String, group_Slug_In: [String], user: ID, user_In: [ID], userRole: CoreMembershipUserRoleChoices, user_Email_Icontains: String, user_Email_In: [String], membershipStatus: CoreMembershipMembershipStatusChoices): MembershipNodeConnection! - associatedLandscapes(offset: Int, before: String, after: String, first: Int, last: Int, landscape: ID, landscape_Slug_Icontains: String, group: ID, group_Slug_Icontains: String, isDefaultLandscapeGroup: Boolean, isPartnership: Boolean): LandscapeGroupNodeConnection! + associatedLandscapes(offset: Int, before: String, after: String, first: Int, last: Int, landscape: ID, landscape_Slug_Icontains: String, group: ID, group_Slug_Icontains: String, isPartnership: Boolean): LandscapeGroupNodeConnection! id: ID! sharedResources(offset: Int, before: String, after: String, first: Int, last: Int, source_DataEntry_ResourceType_In: [String]): SharedResourceNodeConnection accountMembership: MembershipNode @@ -270,7 +270,6 @@ type LandscapeGroupNodeEdge { type LandscapeGroupNode implements Node { landscape: LandscapeNode! group: GroupNode! - isDefaultLandscapeGroup: Boolean! isPartnership: Boolean! partnershipYear: Int id: ID! @@ -294,7 +293,7 @@ type LandscapeNode implements Node { profileImageDescription: String! centerCoordinates: Point associatedDevelopmentStrategy(offset: Int, before: String, after: String, first: Int, last: Int): LandscapeDevelopmentStrategyNodeConnection! - associatedGroups(offset: Int, before: String, after: String, first: Int, last: Int, landscape: ID, landscape_Slug_Icontains: String, group: ID, group_Slug_Icontains: String, isDefaultLandscapeGroup: Boolean, isPartnership: Boolean): LandscapeGroupNodeConnection! + associatedGroups(offset: Int, before: String, after: String, first: Int, last: Int, landscape: ID, landscape_Slug_Icontains: String, group: ID, group_Slug_Icontains: String, isPartnership: Boolean): LandscapeGroupNodeConnection! id: ID! sharedResources(offset: Int, before: String, after: String, first: Int, last: Int, source_DataEntry_ResourceType_In: [String]): SharedResourceNodeConnection areaTypes: [String] diff --git a/terraso_backend/tests/graphql/conftest.py b/terraso_backend/tests/graphql/conftest.py index 1568f93f6..bcf76b790 100644 --- a/terraso_backend/tests/graphql/conftest.py +++ b/terraso_backend/tests/graphql/conftest.py @@ -242,21 +242,18 @@ def memberships_pending_with_notifications(groups, users_with_group_notification @pytest.fixture -def landscape_groups(landscapes, groups): - first_group, second_group = groups[0], groups[1] +def landscape_common_group(landscapes, groups): + group = groups[1] landscape = landscapes[0] - default_group = mixer.blend( - LandscapeGroup, landscape=landscape, group=first_group, is_default_landscape_group=True - ) - common_group = mixer.blend(LandscapeGroup, landscape=landscape, group=second_group) + common_group = mixer.blend(LandscapeGroup, landscape=landscape, group=group) - return [default_group, common_group] + return common_group @pytest.fixture def make_core_db_records( - group_associations, landscapes, landscape_groups, memberships, groups, subgroups, users + group_associations, landscapes, landscape_common_group, memberships, groups, subgroups, users ): return diff --git a/terraso_backend/tests/graphql/mutations/test_landscape_groups_mutations.py b/terraso_backend/tests/graphql/mutations/test_landscape_groups_mutations.py index d1ee13b18..d2879bd2a 100644 --- a/terraso_backend/tests/graphql/mutations/test_landscape_groups_mutations.py +++ b/terraso_backend/tests/graphql/mutations/test_landscape_groups_mutations.py @@ -18,6 +18,8 @@ import pytest from mixer.backend.django import mixer +from apps.collaboration.models import Membership as CollaborationMembership +from apps.core import landscape_collaboration_roles from apps.core.models import Group, LandscapeGroup pytestmark = pytest.mark.django_db @@ -39,7 +41,6 @@ def test_landscape_groups_add_by_landscape_manager(client_query, managed_landsca group { name } - isDefaultLandscapeGroup } } } @@ -56,7 +57,6 @@ def test_landscape_groups_add_by_landscape_manager(client_query, managed_landsca assert landscape_group["id"] assert landscape_group["landscape"]["name"] == landscape.name assert landscape_group["group"]["name"] == group.name - assert not landscape_group["isDefaultLandscapeGroup"] def test_landscape_groups_add_by_non_landscape_manager_not_allowed( @@ -77,7 +77,6 @@ def test_landscape_groups_add_by_non_landscape_manager_not_allowed( group { name } - isDefaultLandscapeGroup } errors } @@ -96,11 +95,12 @@ def test_landscape_groups_add_by_non_landscape_manager_not_allowed( assert "create_not_allowed" in response["data"]["addLandscapeGroup"]["errors"][0]["message"] -def test_landscape_groups_add_duplicated(client_query, users, landscape_groups): +def test_landscape_groups_add_duplicated(client_query, users, landscape_common_group): user = users[0] - landscape = landscape_groups[0].landscape - group = landscape_groups[0].group - group.add_manager(user) + landscape = landscape_common_group.landscape + landscape.membership_list.save_membership( + user.email, landscape_collaboration_roles.ROLE_MANAGER, CollaborationMembership.APPROVED + ) response = client_query( """ @@ -114,7 +114,6 @@ def test_landscape_groups_add_duplicated(client_query, users, landscape_groups): group { name } - isDefaultLandscapeGroup } errors } @@ -123,7 +122,7 @@ def test_landscape_groups_add_duplicated(client_query, users, landscape_groups): variables={ "input": { "landscapeSlug": landscape.slug, - "groupSlug": group.slug, + "groupSlug": landscape_common_group.group.slug, } }, ) @@ -147,7 +146,6 @@ def test_landscape_groups_add_landscape_not_found(client_query, groups): group { name } - isDefaultLandscapeGroup } errors } @@ -181,7 +179,6 @@ def test_landscape_groups_add_group_not_found(client_query, managed_landscapes): group { name } - isDefaultLandscapeGroup } errors } @@ -200,9 +197,9 @@ def test_landscape_groups_add_group_not_found(client_query, managed_landscapes): assert "not_found" in response["data"]["addLandscapeGroup"]["errors"][0]["message"] -def test_landscape_groups_delete_by_group_manager(client_query, users, landscape_groups): +def test_landscape_groups_delete_by_group_manager(client_query, users, landscape_common_group): user = users[0] - _, old_landscape_group = landscape_groups + old_landscape_group = landscape_common_group old_landscape_group.group.add_manager(user) response = client_query( @@ -252,8 +249,10 @@ def test_landscape_groups_delete_by_landscape_manager(client_query, users, manag ) -def test_landscape_groups_delete_by_non_managers_not_allowed(client_query, users, landscape_groups): - _, old_landscape_group = landscape_groups +def test_landscape_groups_delete_by_non_managers_not_allowed( + client_query, users, landscape_common_group +): + old_landscape_group = landscape_common_group response = client_query( """ diff --git a/terraso_backend/tests/graphql/mutations/test_landscape_mutations.py b/terraso_backend/tests/graphql/mutations/test_landscape_mutations.py index e5e1cc0f8..6a8f2d6a3 100644 --- a/terraso_backend/tests/graphql/mutations/test_landscape_mutations.py +++ b/terraso_backend/tests/graphql/mutations/test_landscape_mutations.py @@ -318,7 +318,7 @@ def test_landscapes_update_group_associations(client_query, managed_landscapes, mutation updateLandscape($input: LandscapeUpdateMutationInput!) { updateLandscape(input: $input) { landscape { - associatedGroups(isDefaultLandscapeGroup: false) { + associatedGroups { edges { node { isPartnership @@ -335,7 +335,8 @@ def test_landscapes_update_group_associations(client_query, managed_landscapes, """, variables={"input": new_data}, ) - landscape_result = response.json()["data"]["updateLandscape"]["landscape"] + json_response = response.json() + landscape_result = json_response["data"]["updateLandscape"]["landscape"] def sortedBy(node): return node["node"]["group"]["name"] diff --git a/terraso_backend/tests/graphql/test_groups_filters.py b/terraso_backend/tests/graphql/test_groups_filters.py index 71afb2b03..ca9319961 100644 --- a/terraso_backend/tests/graphql/test_groups_filters.py +++ b/terraso_backend/tests/graphql/test_groups_filters.py @@ -42,10 +42,9 @@ def test_groups_filter_by_membership_user_ignores_deleted_memberships(client_que assert not edges -def test_groups_filter_by_with_landscape_association(client_query, users, landscape_groups): +def test_groups_filter_by_with_landscape_association(client_query, users, landscape_common_group): user = users[0] - default_group_association, common_group_association = landscape_groups - mixer.blend(Membership, user=user, group=default_group_association.group) + common_group_association = landscape_common_group mixer.blend(Membership, user=user, group=common_group_association.group) response = client_query( @@ -66,20 +65,17 @@ def test_groups_filter_by_with_landscape_association(client_query, users, landsc 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 len(groups_result) == 1 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 + client_query, users, landscape_common_group ): 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) + mixer.blend(Membership, user=user, group=landscape_common_group.group) - default_group_association.delete() + landscape_common_group.delete() response = client_query( """ @@ -99,35 +95,7 @@ def test_groups_filter_by_with_landscape_association_ignores_deleted_association 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 + assert len(groups_result) == 0 def test_groups_filter_by_without_landscape_association(client_query, users): diff --git a/terraso_backend/tests/graphql/test_landscape_groups.py b/terraso_backend/tests/graphql/test_landscape_groups.py index a4e8a87f1..6f5877e98 100644 --- a/terraso_backend/tests/graphql/test_landscape_groups.py +++ b/terraso_backend/tests/graphql/test_landscape_groups.py @@ -18,7 +18,7 @@ pytestmark = pytest.mark.django_db -def test_landscape_groups_query(client_query, landscape_groups): +def test_landscape_groups_query(client_query, landscape_common_group): response = client_query( """ {landscapeGroups { @@ -30,7 +30,6 @@ def test_landscape_groups_query(client_query, landscape_groups): group { slug } - isDefaultLandscapeGroup } } }} @@ -44,30 +43,29 @@ def test_landscape_groups_query(client_query, landscape_groups): ] landscapes_and_groups_expected = [ - (lsg.landscape.slug, lsg.group.slug) for lsg in landscape_groups + (landscape_common_group.landscape.slug, landscape_common_group.group.slug) ] assert landscapes_and_groups_expected == landscapes_and_groups_returned -def test_landscape_group_get_one_by_id(client_query, landscape_groups): - landscape_group = landscape_groups[0] +def test_landscape_group_get_one_by_id(client_query, landscape_common_group): query = ( """ {landscapeGroup(id: "%s") { id }} """ - % landscape_group.id + % landscape_common_group.id ) response = client_query(query) landscape_group_response = response.json()["data"]["landscapeGroup"] - assert landscape_group_response["id"] == str(landscape_group.id) + assert landscape_group_response["id"] == str(landscape_common_group.id) -def test_landscape_groups_query_has_total_count(client_query, landscape_groups): +def test_landscape_groups_query_has_total_count(client_query, landscape_common_group): response = client_query( """ {landscapeGroups { @@ -82,4 +80,4 @@ def test_landscape_groups_query_has_total_count(client_query, landscape_groups): ) total_count = response.json()["data"]["landscapeGroups"]["totalCount"] - assert total_count == len(landscape_groups) + assert total_count == 1 diff --git a/terraso_backend/tests/storage/conftest.py b/terraso_backend/tests/storage/conftest.py index e8e3d1e95..abc7b97b7 100644 --- a/terraso_backend/tests/storage/conftest.py +++ b/terraso_backend/tests/storage/conftest.py @@ -17,7 +17,9 @@ from mixer.backend.django import mixer from apps.auth.services import JWTService -from apps.core.models import Group, Landscape, LandscapeGroup, User +from apps.collaboration.models import Membership as CollaborationMembership +from apps.core import landscape_collaboration_roles +from apps.core.models import Landscape, User @pytest.fixture @@ -33,7 +35,9 @@ def user(): @pytest.fixture def landscape(user): landscape = mixer.blend(Landscape) - group = mixer.blend(Group) - group.add_manager(user) - mixer.blend(LandscapeGroup, landscape=landscape, group=group, is_default_landscape_group=True) + landscape.membership_list.save_membership( + user.email, + landscape_collaboration_roles.ROLE_MANAGER, + CollaborationMembership.APPROVED, + ) return landscape diff --git a/terraso_backend/tests/storage/test_views.py b/terraso_backend/tests/storage/test_views.py index ffe3955f5..59480106d 100644 --- a/terraso_backend/tests/storage/test_views.py +++ b/terraso_backend/tests/storage/test_views.py @@ -56,7 +56,9 @@ def test_post_user_profile_image(mock_s3, client, access_token): assert response.status_code == 200 -def test_create_data_entry_successfully(logged_client, landscape_profile_image_payload): +def test_create_landscape_profile_image_successfully( + logged_client, landscape_profile_image_payload +): url = reverse("terraso_storage:landscape-profile-image") with patch( "apps.storage.forms.profile_image_upload_service.upload_file" From 70e474373b792fccfb5628007db4a34108a3a139 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Fri, 6 Oct 2023 17:30:12 -0500 Subject: [PATCH 06/34] refactor: (WIP) landscape add membership tests --- .../apps/collaboration/models/memberships.py | 1 + .../core/landscape_collaboration_roles.py | 2 + terraso_backend/apps/core/permission_rules.py | 12 + .../apps/graphql/schema/__init__.py | 2 + .../apps/graphql/schema/landscapes.py | 124 +++- .../apps/graphql/schema/schema.graphql | 15 + .../test_landscape_membership_mutations.py | 641 ++++++++++++++++++ todo.md | 3 +- 8 files changed, 796 insertions(+), 4 deletions(-) create mode 100644 terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py diff --git a/terraso_backend/apps/collaboration/models/memberships.py b/terraso_backend/apps/collaboration/models/memberships.py index 8e32ac903..227bd43d2 100644 --- a/terraso_backend/apps/collaboration/models/memberships.py +++ b/terraso_backend/apps/collaboration/models/memberships.py @@ -79,6 +79,7 @@ def save_membership( "user_role": user_role, "membership_status": membership_status, "current_membership": membership, + "user_exists": user_exists, } ): raise ValidationError("User cannot request membership") diff --git a/terraso_backend/apps/core/landscape_collaboration_roles.py b/terraso_backend/apps/core/landscape_collaboration_roles.py index e677e6b64..9c53467d9 100644 --- a/terraso_backend/apps/core/landscape_collaboration_roles.py +++ b/terraso_backend/apps/core/landscape_collaboration_roles.py @@ -16,3 +16,5 @@ ROLE_MANAGER = "manager" ROLE_MEMBER = "member" + +ALL_ROLES = [ROLE_MANAGER, ROLE_MEMBER] diff --git a/terraso_backend/apps/core/permission_rules.py b/terraso_backend/apps/core/permission_rules.py index bddca9cca..9edc36fe1 100644 --- a/terraso_backend/apps/core/permission_rules.py +++ b/terraso_backend/apps/core/permission_rules.py @@ -120,6 +120,18 @@ def allowed_to_add_membership(user, group): return group.can_join or group.is_manager(user) +@rules.predicate +def allowed_to_change_landscape_membership(user, obj): + landscape = obj.get("landscape") + user_exists = obj.get("user_exists") + + if not user_exists: + return False + + return user.is_landscape_manager(landscape.id) + + rules.add_rule("allowed_group_managers_count", allowed_group_managers_count) rules.add_rule("allowed_to_update_preferences", allowed_to_update_preferences) rules.add_rule("allowed_to_change_landscape", allowed_to_change_landscape) +rules.add_rule("allowed_to_change_landscape_membership", allowed_to_change_landscape_membership) diff --git a/terraso_backend/apps/graphql/schema/__init__.py b/terraso_backend/apps/graphql/schema/__init__.py index 1186fc8ad..ee0af1bf7 100644 --- a/terraso_backend/apps/graphql/schema/__init__.py +++ b/terraso_backend/apps/graphql/schema/__init__.py @@ -69,6 +69,7 @@ from .landscapes import ( LandscapeAddMutation, LandscapeDeleteMutation, + LandscapeMembershipSaveMutation, LandscapeNode, LandscapeUpdateMutation, ) @@ -202,6 +203,7 @@ class Mutations(graphene.ObjectType): add_site_note = SiteNoteAddMutation.Field() update_site_note = SiteNoteUpdateMutation.Field() delete_site_note = SiteNoteDeleteMutation.Field() + save_landscape_membership = LandscapeMembershipSaveMutation.Field() schema = graphene.Schema(query=Query, mutation=Mutations) diff --git a/terraso_backend/apps/graphql/schema/landscapes.py b/terraso_backend/apps/graphql/schema/landscapes.py index 632d5588a..75f82ba4a 100644 --- a/terraso_backend/apps/graphql/schema/landscapes.py +++ b/terraso_backend/apps/graphql/schema/landscapes.py @@ -14,12 +14,16 @@ # along with this program. If not, see https://www.gnu.org/licenses/. import graphene +import rules import structlog -from django.db import transaction +from django.core.exceptions import NON_FIELD_ERRORS, ValidationError +from django.db import IntegrityError, transaction from django.db.models import Count, Prefetch, Q from graphene import relay from graphene_django import DjangoObjectType +from apps.collaboration.graphql import CollaborationMembershipNode +from apps.core import landscape_collaboration_roles from apps.core.gis.utils import m2_to_hectares from apps.core.models import ( Group, @@ -29,9 +33,18 @@ Membership, TaxonomyTerm, ) -from apps.graphql.exceptions import GraphQLNotAllowedException +from apps.graphql.exceptions import ( + GraphQLNotAllowedException, + GraphQLNotFoundException, + GraphQLValidationException, +) -from .commons import BaseDeleteMutation, BaseWriteMutation, TerrasoConnection +from .commons import ( + BaseAuthenticatedMutation, + BaseDeleteMutation, + BaseWriteMutation, + TerrasoConnection, +) from .constants import MutationTypes from .gis import Point from .shared_resources_mixin import SharedResourcesMixin @@ -321,3 +334,108 @@ def mutate_and_get_payload(cls, root, info, **kwargs): model_name=Landscape.__name__, operation=MutationTypes.DELETE ) return super().mutate_and_get_payload(root, info, **kwargs) + + +class LandscapeMembershipSaveMutation(BaseAuthenticatedMutation): + model_class = Membership + memberships = graphene.Field(graphene.List(CollaborationMembershipNode)) + landscape = graphene.Field(LandscapeNode) + + class Input: + user_role = graphene.String(required=True) + user_emails = graphene.List(graphene.String, required=True) + landscape_slug = graphene.String(required=True) + + @classmethod + @transaction.atomic + def mutate_and_get_payload(cls, root, info, **kwargs): + user = info.context.user + + if kwargs["user_role"] not in landscape_collaboration_roles.ALL_ROLES: + logger.info( + "Attempt to save Landscape Memberships, but user role is not valid", + extra={ + "user_role": kwargs["user_role"], + }, + ) + raise GraphQLNotAllowedException( + model_name=Membership.__name__, operation=MutationTypes.UPDATE + ) + + landscape_slug = kwargs["landscape_slug"] + + try: + landscape = Landscape.objects.get(slug=landscape_slug) + except Exception as error: + logger.error( + "Attempt to save Story Map Memberships, but story map was not found", + extra={ + "landscape_slug": landscape_slug, + "error": error, + }, + ) + raise GraphQLNotFoundException(model_name=Landscape.__name__) + + def validate(context): + if not rules.test_rule( + "allowed_to_change_landscape_membership", + user, + { + "landscape": landscape, + **context, + }, + ): + raise ValidationError("User cannot request membership") + + try: + memberships = [ + { + "membership": result[1], + "was_approved": result[0], + } + for email in kwargs["user_emails"] + for result in [ + landscape.membership_list.save_membership( + user_email=email, + user_role=kwargs["user_role"], + membership_status=Membership.APPROVED, + validation_func=validate, + ) + ] + ] + except ValidationError as error: + logger.error( + "Attempt to save Landscape Memberships, but user is not allowed", + extra={"error": str(error)}, + ) + raise GraphQLNotAllowedException( + model_name=Membership.__name__, operation=MutationTypes.UPDATE + ) + except IntegrityError as exc: + logger.info( + "Attempt to save Landscape Memberships, but it's not unique", + extra={"model": Membership.__name__, "integrity_error": exc}, + ) + + validation_error = ValidationError( + message={ + NON_FIELD_ERRORS: ValidationError( + message=f"This {Membership.__name__} already exists", + code="unique", + ) + }, + ) + raise GraphQLValidationException.from_validation_error( + validation_error, model_name=Membership.__name__ + ) + except Exception as error: + logger.error( + "Attempt to update Story Map Memberships, but there was an error", + extra={"error": str(error)}, + ) + raise GraphQLNotFoundException(model_name=Membership.__name__) + + return cls( + memberships=[membership["membership"] for membership in memberships], + landscape=landscape, + ) diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index 9e78910d2..ae2f46237 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -1690,6 +1690,7 @@ type Mutations { addSiteNote(input: SiteNoteAddMutationInput!): SiteNoteAddMutationPayload! updateSiteNote(input: SiteNoteUpdateMutationInput!): SiteNoteUpdateMutationPayload! deleteSiteNote(input: SiteNoteDeleteMutationInput!): SiteNoteDeleteMutationPayload! + saveLandscapeMembership(input: LandscapeMembershipSaveMutationInput!): LandscapeMembershipSaveMutationPayload! } type GroupAddMutationPayload { @@ -2466,3 +2467,17 @@ input SiteNoteDeleteMutationInput { id: ID! clientMutationId: String } + +type LandscapeMembershipSaveMutationPayload { + errors: GenericScalar + memberships: [CollaborationMembershipNode] + landscape: LandscapeNode + clientMutationId: String +} + +input LandscapeMembershipSaveMutationInput { + userRole: String! + userEmails: [String]! + landscapeSlug: String! + clientMutationId: String +} diff --git a/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py b/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py new file mode 100644 index 000000000..876d3618b --- /dev/null +++ b/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py @@ -0,0 +1,641 @@ +# Copyright © 2021-2023 Technology Matters +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published +# by the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see https://www.gnu.org/licenses/. + +# import uuid +# from unittest import mock + +import pytest + +from apps.collaboration.models import Membership as CollaborationMembership +from apps.core import landscape_collaboration_roles + +# from mixer.backend.django import mixer + + +pytestmark = pytest.mark.django_db + + +def test_landscape_membership_add(client_query, managed_landscapes, users): + landscape = managed_landscapes[0] + user = users[0] + + response = client_query( + """ + mutation addLandscapeMembership($input: LandscapeMembershipSaveMutationInput!){ + saveLandscapeMembership(input: $input) { + memberships { + id + userRole + user { + email + } + } + landscape { + slug + } + errors + } + } + """, + variables={ + "input": { + "userEmails": [user.email], + "userRole": landscape_collaboration_roles.ROLE_MEMBER, + "landscapeSlug": landscape.slug, + } + }, + ) + json_response = response.json() + + membership = json_response["data"]["saveLandscapeMembership"]["memberships"][0] + + assert membership["id"] + assert membership["user"]["email"] == user.email + assert membership["userRole"] == landscape_collaboration_roles.ROLE_MEMBER + + assert json_response["data"]["saveLandscapeMembership"]["landscape"]["slug"] == landscape.slug + + +def test_landscape_membership_add_landscape_not_found(client_query, users): + user = users[0] + + response = client_query( + """ + mutation addLandscapeMembership($input: LandscapeMembershipSaveMutationInput!){ + saveLandscapeMembership(input: $input) { + errors + } + } + """, + variables={ + "input": { + "userEmails": [user.email], + "userRole": landscape_collaboration_roles.ROLE_MEMBER, + "landscapeSlug": "non-existing-landscape", + } + }, + ) + response = response.json() + + assert "errors" in response["data"]["saveLandscapeMembership"] + assert "not_found" in response["data"]["saveLandscapeMembership"]["errors"][0]["message"] + + +def test_landscape_membership_add_user_not_found(client_query, managed_landscapes): + landscape = managed_landscapes[0] + + response = client_query( + """ + mutation addLandscapeMembership($input: LandscapeMembershipSaveMutationInput!){ + saveLandscapeMembership(input: $input) { + errors + } + } + """, + variables={ + "input": { + "userEmails": ["no-existing@user.com"], + "userRole": landscape_collaboration_roles.ROLE_MEMBER, + "landscapeSlug": landscape.slug, + } + }, + ) + response = response.json() + + assert "errors" in response["data"]["saveLandscapeMembership"] + assert ( + "update_not_allowed" in response["data"]["saveLandscapeMembership"]["errors"][0]["message"] + ) + + +# def test_landscape_membership_adding_duplicated_returns_previously_created( +# client_query, managed_landscapes +# ): +# landscape = managed_landscapes[0] +# membership = landscape.membership_list.memberships.first() +# user = membership.user + +# response = client_query( +# """ +# mutation addMembership($input: LandscapeMembershipSaveMutationInput!){ +# saveLandscapeMembership(input: $input) { +# membership { +# id +# userRole +# user { +# email +# } +# group { +# slug +# } +# } +# } +# } +# """, +# variables={ +# "input": { +# "userEmail": user.email, +# "userRole" +# "landscapeSlug": landscape.slug, +# } +# }, +# ) +# membership_response = response.json()["data"]["addMembership"]["membership"] + +# assert membership_response["id"] == str(membership.id) +# assert membership_response["userRole"] == membership.user_role.upper() +# assert membership_response["user"]["email"] == user.email +# assert membership_response["group"]["slug"] == group.slug + + +def test_landscape_membership_add_manager(client_query, managed_landscapes, users): + landscape = managed_landscapes[0] + user = users[0] + + response = client_query( + """ + mutation addMembership($input: LandscapeMembershipSaveMutationInput!){ + saveLandscapeMembership(input: $input) { + memberships { + id + userRole + membershipStatus + user { + email + } + } + } + } + """, + variables={ + "input": { + "userEmails": [user.email], + "landscapeSlug": landscape.slug, + "userRole": landscape_collaboration_roles.ROLE_MANAGER, + } + }, + ) + json_response = response.json() + membership = json_response["data"]["saveLandscapeMembership"]["memberships"][0] + + assert membership["id"] + assert membership["user"]["email"] == user.email + assert membership["userRole"] == landscape_collaboration_roles.ROLE_MANAGER + assert membership["membershipStatus"] == CollaborationMembership.APPROVED.upper() + + +# def test_landscape_membership_add_manager_closed(client_query, groups_closed, users): +# group = groups_closed[0] +# user = users[0] + +# response = client_query( +# """ +# mutation addMembership($input: LandscapeMembershipSaveMutationInput!){ +# saveLandscapeMembership(input: $input) { +# membership { +# id +# userRole +# membershipStatus +# user { +# email +# } +# group { +# slug +# } +# } +# } +# } +# """, +# variables={ +# "input": { +# "userEmail": user.email, +# "groupSlug": group.slug, +# "userRole": Membership.ROLE_MANAGER, +# } +# }, +# ) +# membership = response.json()["data"]["addMembership"]["membership"] + +# assert membership["id"] +# assert membership["user"]["email"] == user.email +# assert membership["group"]["slug"] == group.slug +# assert membership["userRole"] == Membership.ROLE_MANAGER.upper() +# assert membership["membershipStatus"] == Membership.PENDING.upper() + + +# @mock.patch("apps.notifications.email.send_mail") +# def test_landscape_membership_add_member_closed_with_notification( +# mocked_send_mail, client_query, groups_closed, users_with_group_notifications +# ): +# group = groups_closed[0] +# user = users_with_group_notifications[0] +# other_user = users_with_group_notifications[1] + +# group.add_manager(other_user) + +# response = client_query( +# """ +# mutation addMembership($input: LandscapeMembershipSaveMutationInput!){ +# saveLandscapeMembership(input: $input) { +# membership { +# id +# userRole +# membershipStatus +# user { +# email +# } +# group { +# slug +# } +# } +# } +# } +# """, +# variables={ +# "input": { +# "userEmail": user.email, +# "groupSlug": group.slug, +# "userRole": Membership.ROLE_MEMBER, +# } +# }, +# ) +# membership = response.json()["data"]["addMembership"]["membership"] + +# mocked_send_mail.assert_called_once() +# assert mocked_send_mail.call_args.args[3] == [other_user.name_and_email()] + +# assert membership["id"] +# assert membership["user"]["email"] == user.email +# assert membership["group"]["slug"] == group.slug +# assert membership["userRole"] == Membership.ROLE_MEMBER.upper() +# assert membership["membershipStatus"] == Membership.PENDING.upper() + + +# @mock.patch("apps.notifications.email.send_mail") +# def test_landscape_membership_add_member_closed_without_notification( +# mocked_send_mail, client_query, groups_closed, users +# ): +# group = groups_closed[0] +# user = users[0] +# other_user = users[1] + +# group.add_manager(other_user) + +# response = client_query( +# """ +# mutation addMembership($input: LandscapeMembershipSaveMutationInput!){ +# saveLandscapeMembership(input: $input) { +# membership { +# id +# userRole +# membershipStatus +# user { +# email +# } +# group { +# slug +# } +# } +# } +# } +# """, +# variables={ +# "input": { +# "userEmail": user.email, +# "groupSlug": group.slug, +# "userRole": Membership.ROLE_MEMBER, +# } +# }, +# ) +# membership = response.json()["data"]["addMembership"]["membership"] + +# mocked_send_mail.assert_not_called() + +# assert membership["id"] +# assert membership["user"]["email"] == user.email +# assert membership["group"]["slug"] == group.slug +# assert membership["userRole"] == Membership.ROLE_MEMBER.upper() +# assert membership["membershipStatus"] == Membership.PENDING.upper() + + +# @mock.patch("apps.notifications.email.send_mail") +# def test_landscape_membership_update( +# mocked_send_mail, client_query, users, memberships_pending_with_notifications +# ): +# user = users[0] +# other_manager = users[1] +# old_membership = memberships_pending_with_notifications[0] + +# old_membership.group.add_manager(user) +# old_membership.group.add_manager(other_manager) + +# assert old_membership.user_role != Membership.ROLE_MANAGER.upper() +# assert old_membership.membership_status != Membership.PENDING.upper() + +# response = client_query( +# """ +# mutation updateMembership($input: MembershipUpdateMutationInput!){ +# updateMembership(input: $input) { +# membership { +# id +# userRole +# membershipStatus +# user { +# email +# } +# group { +# slug +# } +# } +# } +# } +# """, +# variables={ +# "input": { +# "id": str(old_membership.id), +# "userRole": Membership.ROLE_MANAGER, +# "membershipStatus": Membership.APPROVED, +# } +# }, +# ) +# membership = response.json()["data"]["updateMembership"]["membership"] +# mocked_send_mail.assert_called_once() +# assert mocked_send_mail.call_args.args[3] == [old_membership.user.name_and_email()] + +# assert membership["id"] +# assert membership["user"]["email"] == old_membership.user.email +# assert membership["group"]["slug"] == old_membership.group.slug +# assert membership["userRole"] == Membership.ROLE_MANAGER.upper() +# assert membership["membershipStatus"] == Membership.APPROVED.upper() + + +# def test_landscape_membership_update_role_by_last_manager_fails(client_query, users, memberships): +# user = users[0] +# old_membership = memberships[0] + +# old_membership.group.add_manager(user) + +# assert old_membership.user_role != Membership.ROLE_MANAGER.upper() + +# response = client_query( +# """ +# mutation updateMembership($input: MembershipUpdateMutationInput!){ +# updateMembership(input: $input) { +# membership { +# id +# userRole +# user { +# email +# } +# group { +# slug +# } +# } +# errors +# } +# } +# """, +# variables={ +# "input": { +# "id": str(old_membership.id), +# "userRole": Membership.ROLE_MEMBER, +# } +# }, +# ) +# response = response.json() + +# assert "errors" in response["data"]["updateMembership"] +# assert "update_not_allowed" in response["data"]["updateMembership"]["errors"][0]["message"] + + +# def test_landscape_membership_update_by_non_manager_fail(client_query, memberships): +# old_membership = memberships[0] +# old_membership.user_role = Membership.ROLE_MEMBER +# old_membership.save() + +# response = client_query( +# """ +# mutation updateMembership($input: MembershipUpdateMutationInput!){ +# updateMembership(input: $input) { +# membership { +# userRole +# } +# errors +# } +# } +# """, +# variables={ +# "input": { +# "id": str(old_membership.id), +# "userRole": Membership.ROLE_MANAGER, +# } +# }, +# ) +# response = response.json() + +# assert "errors" in response["data"]["updateMembership"] +# assert "update_not_allowed" in response["data"]["updateMembership"]["errors"][0]["message"] + + +# def test_landscape_membership_update_not_found(client_query, memberships): +# response = client_query( +# """ +# mutation updateMembership($input: MembershipUpdateMutationInput!){ +# updateMembership(input: $input) { +# membership { +# userRole +# } +# errors +# } +# } +# """, +# variables={ +# "input": { +# "id": str(uuid.uuid4()), +# "userRole": Membership.ROLE_MANAGER, +# } +# }, +# ) +# response = response.json() + +# assert "errors" in response["data"]["updateMembership"] +# assert "not_found" in response["data"]["updateMembership"]["errors"][0]["message"] + + +# def test_landscape_membership_delete(client_query, users, groups): +# member = users[0] +# manager = users[1] +# group = groups[0] + +# old_membership = mixer.blend(Membership, user=member, group=group) +# mixer.blend(Membership, user=manager, group=group, user_role=Membership.ROLE_MANAGER) + +# client_query( +# """ +# mutation deleteMembership($input: MembershipDeleteMutationInput!){ +# deleteMembership(input: $input) { +# membership { +# user { +# email +# }, +# group { +# slug +# } +# } +# } +# } +# """, +# variables={ +# "input": { +# "id": str(old_membership.id), +# } +# }, +# ) + +# assert not Membership.objects.filter(user=old_membership.user, group=old_membership.group) + + +# def test_landscape_membership_soft_deleted_can_be_created_again(client_query, memberships): +# old_membership = memberships[0] +# old_membership.delete() + +# response = client_query( +# """ +# mutation addMembership($input: LandscapeMembershipSaveMutationInput!){ +# saveLandscapeMembership(input: $input) { +# membership { +# id +# userRole +# user { +# email +# } +# group { +# slug +# } +# } +# } +# } +# """, +# variables={ +# "input": { +# "userEmail": old_membership.user.email, +# "groupSlug": old_membership.group.slug, +# } +# }, +# ) +# membership = response.json()["data"]["addMembership"]["membership"] + +# assert membership["id"] +# assert membership["user"]["email"] == old_membership.user.email +# assert membership["group"]["slug"] == old_membership.group.slug +# assert membership["userRole"] == Membership.ROLE_MEMBER.upper() + + +# def test_landscape_membership_delete_by_group_manager(client_query, memberships, users): +# # This test tries to delete memberships[1], from user[1] with user[0] as +# # manager from membership group +# old_membership = memberships[1] +# manager = users[0] +# old_membership.group.add_manager(manager) + +# client_query( +# """ +# mutation deleteMembership($input: MembershipDeleteMutationInput!){ +# deleteMembership(input: $input) { +# membership { +# user { +# email +# }, +# group { +# slug +# } +# } +# } +# } +# """, +# variables={ +# "input": { +# "id": str(old_membership.id), +# } +# }, +# ) + +# assert not Membership.objects.filter(user=old_membership.user, group=old_membership.group) + + +# def test_landscape_membership_delete_by_any_other_user(client_query, memberships): +# # Client query runs with user[0] from memberships[0] +# # This test tries to delete memberships[1], from user[1] with user[0] +# old_membership = memberships[1] + +# response = client_query( +# """ +# mutation deleteMembership($input: MembershipDeleteMutationInput!){ +# deleteMembership(input: $input) { +# membership { +# user { +# email +# }, +# group { +# slug +# } +# } +# errors +# } +# } +# """, +# variables={ +# "input": { +# "id": str(old_membership.id), +# } +# }, +# ) + +# response = response.json() + +# assert "errors" in response["data"]["deleteMembership"] +# assert "delete_not_allowed" in response["data"]["deleteMembership"]["errors"][0]["message"] + + +# def test_landscape_membership_delete_by_last_manager(client_query, memberships, users): +# old_membership = memberships[0] + +# response = client_query( +# """ +# mutation deleteMembership($input: MembershipDeleteMutationInput!){ +# deleteMembership(input: $input) { +# membership { +# user { +# email +# }, +# group { +# slug +# } +# } +# errors +# } +# } +# """, +# variables={ +# "input": { +# "id": str(old_membership.id), +# } +# }, +# ) + +# response = response.json() + +# assert "errors" in response["data"]["deleteMembership"] +# assert "delete_not_allowed" in response["data"]["deleteMembership"]["errors"][0]["message"] diff --git a/todo.md b/todo.md index 0ecb5e25d..fbe122b50 100644 --- a/todo.md +++ b/todo.md @@ -1 +1,2 @@ -- Check that only members can see landscape membership list \ No newline at end of file +- Check that only members can see landscape membership list +- Need email for landascape membership? \ No newline at end of file From 824216cae31cfe31f0f77f0c4db06dc29596bd9e Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 10 Oct 2023 11:58:16 -0500 Subject: [PATCH 07/34] fix: Update landsacpe membership test --- terraso_backend/tests/graphql/conftest.py | 24 ++--- .../test_landscape_membership_mutations.py | 89 +++++++++---------- .../tests/graphql/test_landscape.py | 6 +- todo.md | 4 +- 4 files changed, 61 insertions(+), 62 deletions(-) diff --git a/terraso_backend/tests/graphql/conftest.py b/terraso_backend/tests/graphql/conftest.py index bcf76b790..121746a74 100644 --- a/terraso_backend/tests/graphql/conftest.py +++ b/terraso_backend/tests/graphql/conftest.py @@ -118,17 +118,6 @@ def landscape_membership_list(landscapes): return landscape.membership_list -@pytest.fixture -def landscape_user_memberships(landscape_membership_list, users): - return mixer.cycle(2).blend( - CollaborationMembership, - membership_list=landscape_membership_list, - user=(u for u in users), - user_role=landscape_collaboration_roles.ROLE_MEMBER, - pending_email=None, - ) - - @pytest.fixture def managed_landscapes(users): landscapes = mixer.cycle(2).blend(Landscape) @@ -143,6 +132,19 @@ def managed_landscapes(users): return landscapes +@pytest.fixture +def landscape_user_memberships(managed_landscapes, users): + memberships = [ + landscape.membership_list.save_membership( + users[i + 1].email, + landscape_collaboration_roles.ROLE_MEMBER, + CollaborationMembership.APPROVED, + )[1] + for i, landscape in enumerate(managed_landscapes) + ] + return memberships + + @pytest.fixture def groups(): return mixer.cycle(5).blend(Group, membership_status=Group.MEMBERSHIP_TYPE_OPEN) diff --git a/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py b/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py index 876d3618b..073b47708 100644 --- a/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py +++ b/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py @@ -330,55 +330,48 @@ def test_landscape_membership_add_manager(client_query, managed_landscapes, user # assert membership["membershipStatus"] == Membership.PENDING.upper() -# @mock.patch("apps.notifications.email.send_mail") -# def test_landscape_membership_update( -# mocked_send_mail, client_query, users, memberships_pending_with_notifications -# ): -# user = users[0] -# other_manager = users[1] -# old_membership = memberships_pending_with_notifications[0] +def test_landscape_membership_update(client_query, managed_landscapes, landscape_user_memberships): + landscape = managed_landscapes[0] + old_membership = landscape_user_memberships[0] -# old_membership.group.add_manager(user) -# old_membership.group.add_manager(other_manager) + assert old_membership.user_role != landscape_collaboration_roles.ROLE_MANAGER -# assert old_membership.user_role != Membership.ROLE_MANAGER.upper() -# assert old_membership.membership_status != Membership.PENDING.upper() + response = client_query( + """ + mutation updateMembership($input: LandscapeMembershipSaveMutationInput!){ + saveLandscapeMembership(input: $input) { + memberships { + id + userRole + membershipStatus + user { + email + } + } + landscape { + slug + } + } + } + """, + variables={ + "input": { + "landscapeSlug": old_membership.membership_list.landscape.get().slug, + "userEmails": [old_membership.user.email], + "userRole": landscape_collaboration_roles.ROLE_MANAGER, + } + }, + ) + json_response = response.json() -# response = client_query( -# """ -# mutation updateMembership($input: MembershipUpdateMutationInput!){ -# updateMembership(input: $input) { -# membership { -# id -# userRole -# membershipStatus -# user { -# email -# } -# group { -# slug -# } -# } -# } -# } -# """, -# variables={ -# "input": { -# "id": str(old_membership.id), -# "userRole": Membership.ROLE_MANAGER, -# "membershipStatus": Membership.APPROVED, -# } -# }, -# ) -# membership = response.json()["data"]["updateMembership"]["membership"] -# mocked_send_mail.assert_called_once() -# assert mocked_send_mail.call_args.args[3] == [old_membership.user.name_and_email()] + assert json_response["data"]["saveLandscapeMembership"]["landscape"]["slug"] == landscape.slug -# assert membership["id"] -# assert membership["user"]["email"] == old_membership.user.email -# assert membership["group"]["slug"] == old_membership.group.slug -# assert membership["userRole"] == Membership.ROLE_MANAGER.upper() -# assert membership["membershipStatus"] == Membership.APPROVED.upper() + membership = json_response["data"]["saveLandscapeMembership"]["memberships"][0] + + assert membership["id"] + assert membership["user"]["email"] == old_membership.user.email + assert membership["userRole"] == landscape_collaboration_roles.ROLE_MANAGER + assert membership["membershipStatus"] == CollaborationMembership.APPROVED.upper() # def test_landscape_membership_update_role_by_last_manager_fails(client_query, users, memberships): @@ -391,7 +384,7 @@ def test_landscape_membership_add_manager(client_query, managed_landscapes, user # response = client_query( # """ -# mutation updateMembership($input: MembershipUpdateMutationInput!){ +# mutation updateMembership($input: LandscapeMembershipSaveMutationInput!){ # updateMembership(input: $input) { # membership { # id @@ -427,7 +420,7 @@ def test_landscape_membership_add_manager(client_query, managed_landscapes, user # response = client_query( # """ -# mutation updateMembership($input: MembershipUpdateMutationInput!){ +# mutation updateMembership($input: LandscapeMembershipSaveMutationInput!){ # updateMembership(input: $input) { # membership { # userRole @@ -452,7 +445,7 @@ def test_landscape_membership_add_manager(client_query, managed_landscapes, user # def test_landscape_membership_update_not_found(client_query, memberships): # response = client_query( # """ -# mutation updateMembership($input: MembershipUpdateMutationInput!){ +# mutation updateMembership($input: LandscapeMembershipSaveMutationInput!){ # updateMembership(input: $input) { # membership { # userRole diff --git a/terraso_backend/tests/graphql/test_landscape.py b/terraso_backend/tests/graphql/test_landscape.py index c1ec1f3d7..5abd50676 100644 --- a/terraso_backend/tests/graphql/test_landscape.py +++ b/terraso_backend/tests/graphql/test_landscape.py @@ -95,7 +95,9 @@ def test_landscapes_query_with_json_polygon(client_query, landscapes): assert landscape.area_polygon in landscapes_result -def test_landscapes_query_with_membership(client_query, landscapes, landscape_user_memberships): +def test_landscapes_query_with_membership( + client_query, managed_landscapes, landscape_user_memberships +): response = client_query( """ {landscapes(slug: "%s") { @@ -117,7 +119,7 @@ def test_landscapes_query_with_membership(client_query, landscapes, landscape_us } }} """ - % landscapes[0].slug + % managed_landscapes[0].slug ) json_response = response.json() diff --git a/todo.md b/todo.md index fbe122b50..9947860d4 100644 --- a/todo.md +++ b/todo.md @@ -1,2 +1,4 @@ - Check that only members can see landscape membership list -- Need email for landascape membership? \ No newline at end of file +- Need email for landascape membership? +- Prefetch for landscape directory +- Remove direct mebership GQL mutations \ No newline at end of file From d45fb2c655eb0f02a2935ba948ae08322062f888 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 10 Oct 2023 12:46:18 -0500 Subject: [PATCH 08/34] refactor: Delete landscape membership --- terraso_backend/apps/core/permission_rules.py | 12 ++ .../apps/graphql/schema/__init__.py | 2 + .../apps/graphql/schema/landscapes.py | 70 ++++++- .../apps/graphql/schema/schema.graphql | 13 ++ .../test_landscape_membership_mutations.py | 173 +++++++++--------- 5 files changed, 179 insertions(+), 91 deletions(-) diff --git a/terraso_backend/apps/core/permission_rules.py b/terraso_backend/apps/core/permission_rules.py index 9edc36fe1..5209f4327 100644 --- a/terraso_backend/apps/core/permission_rules.py +++ b/terraso_backend/apps/core/permission_rules.py @@ -131,7 +131,19 @@ def allowed_to_change_landscape_membership(user, obj): return user.is_landscape_manager(landscape.id) +@rules.predicate +def allowed_to_delete_landscape_membership(user, obj): + landscape = obj.get("landscape") + membership = obj.get("membership") + + if user.is_landscape_manager(landscape.id): + return True + + return membership.user.email == user.email + + rules.add_rule("allowed_group_managers_count", allowed_group_managers_count) rules.add_rule("allowed_to_update_preferences", allowed_to_update_preferences) rules.add_rule("allowed_to_change_landscape", allowed_to_change_landscape) rules.add_rule("allowed_to_change_landscape_membership", allowed_to_change_landscape_membership) +rules.add_rule("allowed_to_delete_landscape_membership", allowed_to_delete_landscape_membership) diff --git a/terraso_backend/apps/graphql/schema/__init__.py b/terraso_backend/apps/graphql/schema/__init__.py index ee0af1bf7..ca1f807b0 100644 --- a/terraso_backend/apps/graphql/schema/__init__.py +++ b/terraso_backend/apps/graphql/schema/__init__.py @@ -69,6 +69,7 @@ from .landscapes import ( LandscapeAddMutation, LandscapeDeleteMutation, + LandscapeMembershipDeleteMutation, LandscapeMembershipSaveMutation, LandscapeNode, LandscapeUpdateMutation, @@ -204,6 +205,7 @@ class Mutations(graphene.ObjectType): update_site_note = SiteNoteUpdateMutation.Field() delete_site_note = SiteNoteDeleteMutation.Field() save_landscape_membership = LandscapeMembershipSaveMutation.Field() + delete_landscape_membership = LandscapeMembershipDeleteMutation.Field() schema = graphene.Schema(query=Query, mutation=Mutations) diff --git a/terraso_backend/apps/graphql/schema/landscapes.py b/terraso_backend/apps/graphql/schema/landscapes.py index 75f82ba4a..f6794678e 100644 --- a/terraso_backend/apps/graphql/schema/landscapes.py +++ b/terraso_backend/apps/graphql/schema/landscapes.py @@ -23,6 +23,7 @@ from graphene_django import DjangoObjectType from apps.collaboration.graphql import CollaborationMembershipNode +from apps.collaboration.models import Membership as CollaborationMembership from apps.core import landscape_collaboration_roles from apps.core.gis.utils import m2_to_hectares from apps.core.models import ( @@ -337,7 +338,7 @@ def mutate_and_get_payload(cls, root, info, **kwargs): class LandscapeMembershipSaveMutation(BaseAuthenticatedMutation): - model_class = Membership + model_class = CollaborationMembership memberships = graphene.Field(graphene.List(CollaborationMembershipNode)) landscape = graphene.Field(LandscapeNode) @@ -359,7 +360,7 @@ def mutate_and_get_payload(cls, root, info, **kwargs): }, ) raise GraphQLNotAllowedException( - model_name=Membership.__name__, operation=MutationTypes.UPDATE + model_name=CollaborationMembership.__name__, operation=MutationTypes.UPDATE ) landscape_slug = kwargs["landscape_slug"] @@ -398,7 +399,7 @@ def validate(context): landscape.membership_list.save_membership( user_email=email, user_role=kwargs["user_role"], - membership_status=Membership.APPROVED, + membership_status=CollaborationMembership.APPROVED, validation_func=validate, ) ] @@ -409,33 +410,86 @@ def validate(context): extra={"error": str(error)}, ) raise GraphQLNotAllowedException( - model_name=Membership.__name__, operation=MutationTypes.UPDATE + model_name=CollaborationMembership.__name__, operation=MutationTypes.UPDATE ) except IntegrityError as exc: logger.info( "Attempt to save Landscape Memberships, but it's not unique", - extra={"model": Membership.__name__, "integrity_error": exc}, + extra={"model": CollaborationMembership.__name__, "integrity_error": exc}, ) validation_error = ValidationError( message={ NON_FIELD_ERRORS: ValidationError( - message=f"This {Membership.__name__} already exists", + message=f"This {CollaborationMembership.__name__} already exists", code="unique", ) }, ) raise GraphQLValidationException.from_validation_error( - validation_error, model_name=Membership.__name__ + validation_error, model_name=CollaborationMembership.__name__ ) except Exception as error: logger.error( "Attempt to update Story Map Memberships, but there was an error", extra={"error": str(error)}, ) - raise GraphQLNotFoundException(model_name=Membership.__name__) + raise GraphQLNotFoundException(model_name=CollaborationMembership.__name__) return cls( memberships=[membership["membership"] for membership in memberships], landscape=landscape, ) + + +class LandscapeMembershipDeleteMutation(BaseDeleteMutation): + membership = graphene.Field(CollaborationMembershipNode) + + model_class = CollaborationMembership + + class Input: + id = graphene.ID(required=True) + landscape_slug = graphene.String(required=True) + + @classmethod + def mutate_and_get_payload(cls, root, info, **kwargs): + user = info.context.user + membership_id = kwargs["id"] + landscape_slug = kwargs["landscape_slug"] + + try: + landscape = Landscape.objects.get(slug=landscape_slug) + except Landscape.DoesNotExist: + logger.error( + "Attempt to delete Landscape Membership, but landscape was not found", + extra={"landscape_slug": landscape_slug}, + ) + raise GraphQLNotFoundException(model_name=Landscape.__name__) + + try: + membership = landscape.membership_list.memberships.get(id=membership_id) + except CollaborationMembership.DoesNotExist: + logger.error( + "Attempt to delete Landscape Membership, but it was not found", + extra={"membership_id": membership_id}, + ) + raise GraphQLNotFoundException(model_name=CollaborationMembership.__name__) + + if not rules.test_rule( + "allowed_to_delete_landscape_membership", + user, + { + "landscape": landscape, + "membership": membership, + }, + ): + logger.info( + "Attempt to delete Landscape Memberships, but user has no permission", + extra={"user_id": user.pk, "membership_id": membership_id}, + ) + raise GraphQLNotAllowedException( + model_name=CollaborationMembership.__name__, operation=MutationTypes.DELETE + ) + + print(f"kwargs: {kwargs}") + return super().mutate_and_get_payload(root, info, **kwargs) diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index ae2f46237..f4134dfcc 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -1691,6 +1691,7 @@ type Mutations { updateSiteNote(input: SiteNoteUpdateMutationInput!): SiteNoteUpdateMutationPayload! deleteSiteNote(input: SiteNoteDeleteMutationInput!): SiteNoteDeleteMutationPayload! saveLandscapeMembership(input: LandscapeMembershipSaveMutationInput!): LandscapeMembershipSaveMutationPayload! + deleteLandscapeMembership(input: LandscapeMembershipDeleteMutationInput!): LandscapeMembershipDeleteMutationPayload! } type GroupAddMutationPayload { @@ -2481,3 +2482,15 @@ input LandscapeMembershipSaveMutationInput { landscapeSlug: String! clientMutationId: String } + +type LandscapeMembershipDeleteMutationPayload { + errors: GenericScalar + membership: CollaborationMembershipNode + clientMutationId: String +} + +input LandscapeMembershipDeleteMutationInput { + id: ID! + landscapeSlug: String! + clientMutationId: String +} diff --git a/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py b/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py index 073b47708..f8362b23b 100644 --- a/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py +++ b/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py @@ -374,43 +374,49 @@ def test_landscape_membership_update(client_query, managed_landscapes, landscape assert membership["membershipStatus"] == CollaborationMembership.APPROVED.upper() -# def test_landscape_membership_update_role_by_last_manager_fails(client_query, users, memberships): -# user = users[0] -# old_membership = memberships[0] +def test_landscape_membership_update_by_non_manager_fail( + client_query, users, managed_landscapes, landscape_user_memberships +): + current_manager = users[0] + landscape = managed_landscapes[0] -# old_membership.group.add_manager(user) + membership = landscape_user_memberships[0] -# assert old_membership.user_role != Membership.ROLE_MANAGER.upper() + landscape.membership_list.save_membership( + current_manager.email, + landscape_collaboration_roles.ROLE_MEMBER, + CollaborationMembership.APPROVED, + ) -# response = client_query( -# """ -# mutation updateMembership($input: LandscapeMembershipSaveMutationInput!){ -# updateMembership(input: $input) { -# membership { -# id -# userRole -# user { -# email -# } -# group { -# slug -# } -# } -# errors -# } -# } -# """, -# variables={ -# "input": { -# "id": str(old_membership.id), -# "userRole": Membership.ROLE_MEMBER, -# } -# }, -# ) -# response = response.json() + response = client_query( + """ + mutation updateMembership($input: LandscapeMembershipSaveMutationInput!){ + saveLandscapeMembership(input: $input) { + memberships { + id + userRole + user { + email + } + } + errors + } + } + """, + variables={ + "input": { + "landscapeSlug": landscape.slug, + "userEmails": [membership.user.email], + "userRole": landscape_collaboration_roles.ROLE_MEMBER, + } + }, + ) + response = response.json() -# assert "errors" in response["data"]["updateMembership"] -# assert "update_not_allowed" in response["data"]["updateMembership"]["errors"][0]["message"] + assert "errors" in response["data"]["saveLandscapeMembership"] + assert ( + "update_not_allowed" in response["data"]["saveLandscapeMembership"]["errors"][0]["message"] + ) # def test_landscape_membership_update_by_non_manager_fail(client_query, memberships): @@ -442,62 +448,63 @@ def test_landscape_membership_update(client_query, managed_landscapes, landscape # assert "update_not_allowed" in response["data"]["updateMembership"]["errors"][0]["message"] -# def test_landscape_membership_update_not_found(client_query, memberships): -# response = client_query( -# """ -# mutation updateMembership($input: LandscapeMembershipSaveMutationInput!){ -# updateMembership(input: $input) { -# membership { -# userRole -# } -# errors -# } -# } -# """, -# variables={ -# "input": { -# "id": str(uuid.uuid4()), -# "userRole": Membership.ROLE_MANAGER, -# } -# }, -# ) -# response = response.json() +def test_landscape_membership_update_not_found(client_query, managed_landscapes): + response = client_query( + """ + mutation updateMembership($input: LandscapeMembershipSaveMutationInput!){ + saveLandscapeMembership(input: $input) { + memberships { + userRole + } + errors + } + } + """, + variables={ + "input": { + "landscapeSlug": "non-existing-landscape", + "userEmails": ["useremail@test.com"], + "userRole": landscape_collaboration_roles.ROLE_MANAGER, + } + }, + ) + response = response.json() -# assert "errors" in response["data"]["updateMembership"] -# assert "not_found" in response["data"]["updateMembership"]["errors"][0]["message"] + assert "errors" in response["data"]["saveLandscapeMembership"] + assert "not_found" in response["data"]["saveLandscapeMembership"]["errors"][0]["message"] -# def test_landscape_membership_delete(client_query, users, groups): -# member = users[0] -# manager = users[1] -# group = groups[0] +def test_landscape_membership_delete(client_query, managed_landscapes, landscape_user_memberships): + landscape = managed_landscapes[0] + old_membership = landscape_user_memberships[0] -# old_membership = mixer.blend(Membership, user=member, group=group) -# mixer.blend(Membership, user=manager, group=group, user_role=Membership.ROLE_MANAGER) + assert CollaborationMembership.objects.filter( + user=old_membership.user, membership_list__landscape=landscape + ) -# client_query( -# """ -# mutation deleteMembership($input: MembershipDeleteMutationInput!){ -# deleteMembership(input: $input) { -# membership { -# user { -# email -# }, -# group { -# slug -# } -# } -# } -# } -# """, -# variables={ -# "input": { -# "id": str(old_membership.id), -# } -# }, -# ) + client_query( + """ + mutation deleteMembership($input: LandscapeMembershipDeleteMutationInput!){ + deleteLandscapeMembership(input: $input) { + membership { + user { + email + }, + } + } + } + """, + variables={ + "input": { + "landscapeSlug": landscape.slug, + "id": str(old_membership.id), + } + }, + ) -# assert not Membership.objects.filter(user=old_membership.user, group=old_membership.group) + assert not CollaborationMembership.objects.filter( + user=old_membership.user, membership_list__landscape=landscape + ) # def test_landscape_membership_soft_deleted_can_be_created_again(client_query, memberships): From 17274d7556b56d70e559dde124f9de6bce96bc99 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 10 Oct 2023 14:10:35 -0500 Subject: [PATCH 09/34] fix: delete and duplicated tests --- .../test_landscape_membership_mutations.py | 477 ++++++------------ 1 file changed, 144 insertions(+), 333 deletions(-) diff --git a/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py b/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py index f8362b23b..81b666b72 100644 --- a/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py +++ b/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py @@ -120,44 +120,39 @@ def test_landscape_membership_add_user_not_found(client_query, managed_landscape ) -# def test_landscape_membership_adding_duplicated_returns_previously_created( -# client_query, managed_landscapes -# ): -# landscape = managed_landscapes[0] -# membership = landscape.membership_list.memberships.first() -# user = membership.user - -# response = client_query( -# """ -# mutation addMembership($input: LandscapeMembershipSaveMutationInput!){ -# saveLandscapeMembership(input: $input) { -# membership { -# id -# userRole -# user { -# email -# } -# group { -# slug -# } -# } -# } -# } -# """, -# variables={ -# "input": { -# "userEmail": user.email, -# "userRole" -# "landscapeSlug": landscape.slug, -# } -# }, -# ) -# membership_response = response.json()["data"]["addMembership"]["membership"] - -# assert membership_response["id"] == str(membership.id) -# assert membership_response["userRole"] == membership.user_role.upper() -# assert membership_response["user"]["email"] == user.email -# assert membership_response["group"]["slug"] == group.slug +def test_landscape_membership_adding_duplicated_returns_existing_membership( + client_query, managed_landscapes, landscape_user_memberships +): + landscape = managed_landscapes[0] + membership = landscape_user_memberships[0] + + response = client_query( + """ + mutation addMembership($input: LandscapeMembershipSaveMutationInput!){ + saveLandscapeMembership(input: $input) { + memberships { + id + userRole + user { + email + } + } + } + } + """, + variables={ + "input": { + "userEmails": [membership.user.email], + "userRole": membership.user_role, + "landscapeSlug": landscape.slug, + } + }, + ) + membership_response = response.json()["data"]["saveLandscapeMembership"]["memberships"][0] + + assert membership_response["id"] == str(membership.id) + assert membership_response["userRole"] == membership.user_role + assert membership_response["user"]["email"] == membership.user.email def test_landscape_membership_add_manager(client_query, managed_landscapes, users): @@ -196,140 +191,6 @@ def test_landscape_membership_add_manager(client_query, managed_landscapes, user assert membership["membershipStatus"] == CollaborationMembership.APPROVED.upper() -# def test_landscape_membership_add_manager_closed(client_query, groups_closed, users): -# group = groups_closed[0] -# user = users[0] - -# response = client_query( -# """ -# mutation addMembership($input: LandscapeMembershipSaveMutationInput!){ -# saveLandscapeMembership(input: $input) { -# membership { -# id -# userRole -# membershipStatus -# user { -# email -# } -# group { -# slug -# } -# } -# } -# } -# """, -# variables={ -# "input": { -# "userEmail": user.email, -# "groupSlug": group.slug, -# "userRole": Membership.ROLE_MANAGER, -# } -# }, -# ) -# membership = response.json()["data"]["addMembership"]["membership"] - -# assert membership["id"] -# assert membership["user"]["email"] == user.email -# assert membership["group"]["slug"] == group.slug -# assert membership["userRole"] == Membership.ROLE_MANAGER.upper() -# assert membership["membershipStatus"] == Membership.PENDING.upper() - - -# @mock.patch("apps.notifications.email.send_mail") -# def test_landscape_membership_add_member_closed_with_notification( -# mocked_send_mail, client_query, groups_closed, users_with_group_notifications -# ): -# group = groups_closed[0] -# user = users_with_group_notifications[0] -# other_user = users_with_group_notifications[1] - -# group.add_manager(other_user) - -# response = client_query( -# """ -# mutation addMembership($input: LandscapeMembershipSaveMutationInput!){ -# saveLandscapeMembership(input: $input) { -# membership { -# id -# userRole -# membershipStatus -# user { -# email -# } -# group { -# slug -# } -# } -# } -# } -# """, -# variables={ -# "input": { -# "userEmail": user.email, -# "groupSlug": group.slug, -# "userRole": Membership.ROLE_MEMBER, -# } -# }, -# ) -# membership = response.json()["data"]["addMembership"]["membership"] - -# mocked_send_mail.assert_called_once() -# assert mocked_send_mail.call_args.args[3] == [other_user.name_and_email()] - -# assert membership["id"] -# assert membership["user"]["email"] == user.email -# assert membership["group"]["slug"] == group.slug -# assert membership["userRole"] == Membership.ROLE_MEMBER.upper() -# assert membership["membershipStatus"] == Membership.PENDING.upper() - - -# @mock.patch("apps.notifications.email.send_mail") -# def test_landscape_membership_add_member_closed_without_notification( -# mocked_send_mail, client_query, groups_closed, users -# ): -# group = groups_closed[0] -# user = users[0] -# other_user = users[1] - -# group.add_manager(other_user) - -# response = client_query( -# """ -# mutation addMembership($input: LandscapeMembershipSaveMutationInput!){ -# saveLandscapeMembership(input: $input) { -# membership { -# id -# userRole -# membershipStatus -# user { -# email -# } -# group { -# slug -# } -# } -# } -# } -# """, -# variables={ -# "input": { -# "userEmail": user.email, -# "groupSlug": group.slug, -# "userRole": Membership.ROLE_MEMBER, -# } -# }, -# ) -# membership = response.json()["data"]["addMembership"]["membership"] - -# mocked_send_mail.assert_not_called() - -# assert membership["id"] -# assert membership["user"]["email"] == user.email -# assert membership["group"]["slug"] == group.slug -# assert membership["userRole"] == Membership.ROLE_MEMBER.upper() -# assert membership["membershipStatus"] == Membership.PENDING.upper() - - def test_landscape_membership_update(client_query, managed_landscapes, landscape_user_memberships): landscape = managed_landscapes[0] old_membership = landscape_user_memberships[0] @@ -419,35 +280,6 @@ def test_landscape_membership_update_by_non_manager_fail( ) -# def test_landscape_membership_update_by_non_manager_fail(client_query, memberships): -# old_membership = memberships[0] -# old_membership.user_role = Membership.ROLE_MEMBER -# old_membership.save() - -# response = client_query( -# """ -# mutation updateMembership($input: LandscapeMembershipSaveMutationInput!){ -# updateMembership(input: $input) { -# membership { -# userRole -# } -# errors -# } -# } -# """, -# variables={ -# "input": { -# "id": str(old_membership.id), -# "userRole": Membership.ROLE_MANAGER, -# } -# }, -# ) -# response = response.json() - -# assert "errors" in response["data"]["updateMembership"] -# assert "update_not_allowed" in response["data"]["updateMembership"]["errors"][0]["message"] - - def test_landscape_membership_update_not_found(client_query, managed_landscapes): response = client_query( """ @@ -507,135 +339,114 @@ def test_landscape_membership_delete(client_query, managed_landscapes, landscape ) -# def test_landscape_membership_soft_deleted_can_be_created_again(client_query, memberships): -# old_membership = memberships[0] -# old_membership.delete() - -# response = client_query( -# """ -# mutation addMembership($input: LandscapeMembershipSaveMutationInput!){ -# saveLandscapeMembership(input: $input) { -# membership { -# id -# userRole -# user { -# email -# } -# group { -# slug -# } -# } -# } -# } -# """, -# variables={ -# "input": { -# "userEmail": old_membership.user.email, -# "groupSlug": old_membership.group.slug, -# } -# }, -# ) -# membership = response.json()["data"]["addMembership"]["membership"] - -# assert membership["id"] -# assert membership["user"]["email"] == old_membership.user.email -# assert membership["group"]["slug"] == old_membership.group.slug -# assert membership["userRole"] == Membership.ROLE_MEMBER.upper() - - -# def test_landscape_membership_delete_by_group_manager(client_query, memberships, users): -# # This test tries to delete memberships[1], from user[1] with user[0] as -# # manager from membership group -# old_membership = memberships[1] -# manager = users[0] -# old_membership.group.add_manager(manager) - -# client_query( -# """ -# mutation deleteMembership($input: MembershipDeleteMutationInput!){ -# deleteMembership(input: $input) { -# membership { -# user { -# email -# }, -# group { -# slug -# } -# } -# } -# } -# """, -# variables={ -# "input": { -# "id": str(old_membership.id), -# } -# }, -# ) - -# assert not Membership.objects.filter(user=old_membership.user, group=old_membership.group) - - -# def test_landscape_membership_delete_by_any_other_user(client_query, memberships): -# # Client query runs with user[0] from memberships[0] -# # This test tries to delete memberships[1], from user[1] with user[0] -# old_membership = memberships[1] - -# response = client_query( -# """ -# mutation deleteMembership($input: MembershipDeleteMutationInput!){ -# deleteMembership(input: $input) { -# membership { -# user { -# email -# }, -# group { -# slug -# } -# } -# errors -# } -# } -# """, -# variables={ -# "input": { -# "id": str(old_membership.id), -# } -# }, -# ) - -# response = response.json() - -# assert "errors" in response["data"]["deleteMembership"] -# assert "delete_not_allowed" in response["data"]["deleteMembership"]["errors"][0]["message"] - - -# def test_landscape_membership_delete_by_last_manager(client_query, memberships, users): -# old_membership = memberships[0] - -# response = client_query( -# """ -# mutation deleteMembership($input: MembershipDeleteMutationInput!){ -# deleteMembership(input: $input) { -# membership { -# user { -# email -# }, -# group { -# slug -# } -# } -# errors -# } -# } -# """, -# variables={ -# "input": { -# "id": str(old_membership.id), -# } -# }, -# ) - -# response = response.json() - -# assert "errors" in response["data"]["deleteMembership"] -# assert "delete_not_allowed" in response["data"]["deleteMembership"]["errors"][0]["message"] +def test_landscape_membership_soft_deleted_can_be_created_again( + client_query, landscape_user_memberships, managed_landscapes +): + landscape = managed_landscapes[0] + old_membership = landscape_user_memberships[0] + old_membership.delete() + + assert not landscape.membership_list.memberships.filter( + user=old_membership.user, deleted_at=None + ).exists() + + response = client_query( + """ + mutation addMembership($input: LandscapeMembershipSaveMutationInput!){ + saveLandscapeMembership(input: $input) { + memberships { + id + userRole + user { + email + } + } + } + } + """, + variables={ + "input": { + "userEmails": [old_membership.user.email], + "landscapeSlug": landscape.slug, + "userRole": landscape_collaboration_roles.ROLE_MEMBER, + } + }, + ) + membership = response.json()["data"]["saveLandscapeMembership"]["memberships"][0] + + assert membership["id"] + assert membership["user"]["email"] == old_membership.user.email + assert membership["userRole"] == landscape_collaboration_roles.ROLE_MEMBER + assert landscape.membership_list.memberships.filter( + user=old_membership.user, deleted_at=None + ).exists() + + +def test_landscape_membership_delete_by_membership_owner(client_query, users, managed_landscapes): + landscape = managed_landscapes[0] + old_membership = landscape.membership_list.memberships.filter( + user=users[0], deleted_at=None + ).first() + + old_membership.user_role = landscape_collaboration_roles.ROLE_MEMBER + old_membership.save() + + client_query( + """ + mutation deleteMembership($input: LandscapeMembershipDeleteMutationInput!){ + deleteLandscapeMembership(input: $input) { + membership { + user { + email + }, + } + } + } + """, + variables={ + "input": { + "id": str(old_membership.id), + "landscapeSlug": landscape.slug, + } + }, + ) + + assert not CollaborationMembership.objects.filter( + user=old_membership.user, membership_list__landscape=landscape + ) + + +def test_landscape_membership_delete_by_any_other_user( + client_query, landscape_user_memberships, managed_landscapes +): + old_membership = landscape_user_memberships[1] + landscape = managed_landscapes[1] + + response = client_query( + """ + mutation deleteMembership($input: LandscapeMembershipDeleteMutationInput!){ + deleteLandscapeMembership(input: $input) { + membership { + user { + email + }, + } + errors + } + } + """, + variables={ + "input": { + "id": str(old_membership.id), + "landscapeSlug": landscape.slug, + } + }, + ) + + response = response.json() + + assert "errors" in response["data"]["deleteLandscapeMembership"] + assert ( + "delete_not_allowed" + in response["data"]["deleteLandscapeMembership"]["errors"][0]["message"] + ) From 0bf3e151a3e7ed45d4ef7727f3b2ac1c636c888f Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 10 Oct 2023 15:21:35 -0500 Subject: [PATCH 10/34] test: Added non members landscape membership test --- .../tests/graphql/test_landscape.py | 35 +++++++++++++++++++ todo.md | 7 ++-- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/terraso_backend/tests/graphql/test_landscape.py b/terraso_backend/tests/graphql/test_landscape.py index 5abd50676..a6902518e 100644 --- a/terraso_backend/tests/graphql/test_landscape.py +++ b/terraso_backend/tests/graphql/test_landscape.py @@ -128,3 +128,38 @@ def test_landscapes_query_with_membership( "memberships" ]["edges"] assert len(memberships) == 2 + + +def test_landscapes_query_with_membership_no_results_non_member( + client_query, managed_landscapes, landscape_user_memberships +): + response = client_query( + """ + {landscapes(slug: "%s") { + edges { + node { + name + membershipList { + memberships { + edges { + node { + user { + email + } + } + } + } + } + } + } + }} + """ + % managed_landscapes[1].slug + ) + + json_response = response.json() + + memberships = json_response["data"]["landscapes"]["edges"][0]["node"]["membershipList"][ + "memberships" + ]["edges"] + assert len(memberships) == 2 diff --git a/todo.md b/todo.md index 9947860d4..43a6993b0 100644 --- a/todo.md +++ b/todo.md @@ -1,4 +1,5 @@ -- Check that only members can see landscape membership list -- Need email for landascape membership? +# TODO +- migrate data to new landscape membership model - Prefetch for landscape directory -- Remove direct mebership GQL mutations \ No newline at end of file +- Need logged in user to add landscape membership? +- Remove direct membership GQL mutations \ No newline at end of file From 6879646786dc93ec0f3b1d3bf714d1aab5b8c1b2 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 10 Oct 2023 15:41:14 -0500 Subject: [PATCH 11/34] fix: Anonymous user membership data --- .../apps/graphql/schema/landscapes.py | 1 - .../tests/graphql/test_landscape.py | 52 ++++++++++++++++--- 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/terraso_backend/apps/graphql/schema/landscapes.py b/terraso_backend/apps/graphql/schema/landscapes.py index f6794678e..da7fe39ca 100644 --- a/terraso_backend/apps/graphql/schema/landscapes.py +++ b/terraso_backend/apps/graphql/schema/landscapes.py @@ -491,5 +491,4 @@ def mutate_and_get_payload(cls, root, info, **kwargs): model_name=CollaborationMembership.__name__, operation=MutationTypes.DELETE ) - print(f"kwargs: {kwargs}") return super().mutate_and_get_payload(root, info, **kwargs) diff --git a/terraso_backend/tests/graphql/test_landscape.py b/terraso_backend/tests/graphql/test_landscape.py index a6902518e..862128488 100644 --- a/terraso_backend/tests/graphql/test_landscape.py +++ b/terraso_backend/tests/graphql/test_landscape.py @@ -105,6 +105,7 @@ def test_landscapes_query_with_membership( node { name membershipList { + membershipsCount memberships { edges { node { @@ -124,13 +125,13 @@ def test_landscapes_query_with_membership( json_response = response.json() - memberships = json_response["data"]["landscapes"]["edges"][0]["node"]["membershipList"][ - "memberships" - ]["edges"] + membership_list = json_response["data"]["landscapes"]["edges"][0]["node"]["membershipList"] + memberships = membership_list["memberships"]["edges"] assert len(memberships) == 2 + assert membership_list["membershipsCount"] == 2 -def test_landscapes_query_with_membership_no_results_non_member( +def test_landscapes_query_with_membership_for_non_member( client_query, managed_landscapes, landscape_user_memberships ): response = client_query( @@ -140,6 +141,7 @@ def test_landscapes_query_with_membership_no_results_non_member( node { name membershipList { + membershipsCount memberships { edges { node { @@ -159,7 +161,43 @@ def test_landscapes_query_with_membership_no_results_non_member( json_response = response.json() - memberships = json_response["data"]["landscapes"]["edges"][0]["node"]["membershipList"][ - "memberships" - ]["edges"] + membership_list = json_response["data"]["landscapes"]["edges"][0]["node"]["membershipList"] + memberships = membership_list["memberships"]["edges"] assert len(memberships) == 2 + assert membership_list["membershipsCount"] == 2 + + +def test_landscapes_query_with_membership_for_anonymous_user( + client_query_no_token, managed_landscapes, landscape_user_memberships +): + response = client_query_no_token( + """ + {landscapes(slug: "%s") { + edges { + node { + name + membershipList { + membershipsCount + memberships { + edges { + node { + user { + email + } + } + } + } + } + } + } + }} + """ + % managed_landscapes[1].slug + ) + + json_response = response.json() + + membership_list = json_response["data"]["landscapes"]["edges"][0]["node"]["membershipList"] + memberships = membership_list["memberships"]["edges"] + assert len(memberships) == 0 + assert membership_list["membershipsCount"] == 0 From 582557d64727d03b37604e1d612b13bd4fe1bb36 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 10 Oct 2023 15:45:03 -0500 Subject: [PATCH 12/34] fix: Optional membership_list for landscapes --- terraso_backend/apps/core/models/landscapes.py | 1 + terraso_backend/apps/graphql/schema/schema.graphql | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/terraso_backend/apps/core/models/landscapes.py b/terraso_backend/apps/core/models/landscapes.py index 00493acfb..a78ec82d2 100644 --- a/terraso_backend/apps/core/models/landscapes.py +++ b/terraso_backend/apps/core/models/landscapes.py @@ -70,6 +70,7 @@ class Landscape(SlugModel, DirtyFieldsMixin): "collaboration.MembershipList", on_delete=models.CASCADE, related_name="landscape", + null=True, ) area_types = models.JSONField(blank=True, null=True) diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index f4134dfcc..5e31bbb73 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -285,7 +285,7 @@ type LandscapeNode implements Node { email: String! areaScalarM2: Float createdBy: UserNode - membershipList: CollaborationMembershipListNode! + membershipList: CollaborationMembershipListNode taxonomyTerms(offset: Int, before: String, after: String, first: Int, last: Int, type: CoreTaxonomyTermTypeChoices, type_In: [CoreTaxonomyTermTypeChoices]): TaxonomyTermNodeConnection! population: Int partnershipStatus: CoreLandscapePartnershipStatusChoices From 2e71137cacf20a7d8b6354a105f82835d393d529 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 10 Oct 2023 16:43:36 -0500 Subject: [PATCH 13/34] refactor: Migration to copy landscape membership data to new model --- .../0046_landscape_membership_list.py | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 terraso_backend/apps/core/migrations/0046_landscape_membership_list.py diff --git a/terraso_backend/apps/core/migrations/0046_landscape_membership_list.py b/terraso_backend/apps/core/migrations/0046_landscape_membership_list.py new file mode 100644 index 000000000..fa65b638e --- /dev/null +++ b/terraso_backend/apps/core/migrations/0046_landscape_membership_list.py @@ -0,0 +1,71 @@ +# Copyright © 2023 Technology Matters +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published +# by the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see https://www.gnu.org/licenses/. + +# Generated by Django 4.2.6 on 2023-10-10 20:45 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + +import apps.core.models.commons + + +def copy_memberships(apps, schema_editor): + Landscape = apps.get_model("core", "Landscape") + MembershipList = apps.get_model("collaboration", "MembershipList") + Membership = apps.get_model("collaboration", "Membership") + landscapes = Landscape.objects.all() + for landscape in landscapes: + if not landscape.membership_list: + landscape.membership_list = MembershipList.objects.create( + enroll_method="both", + membership_type="open", + ) + landscape.save() + default_group = landscape.associated_groups.filter(is_default_landscape_group=True).first() + if not default_group: + print(f"no default group for landscape {landscape.name}") + continue + current_memberships = default_group.group.memberships.filter( + deleted_at__isnull=True + ).distinct("user") + for membership in current_memberships: + Membership.objects.create( + membership_list=landscape.membership_list, + user=membership.user, + user_role=membership.user_role, + membership_status=membership.membership_status, + ) + + +class Migration(migrations.Migration): + dependencies = [ + ("collaboration", "0005_change_collaborator_to_editor"), + ("core", "0045_taxonomyterms_ecosystems_renamed"), + ] + + operations = [ + migrations.AddField( + model_name="landscape", + name="membership_list", + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="landscape", + to="collaboration.membershiplist", + ), + ), + migrations.RunPython(copy_memberships), + ] From 03f0931410b23c395e52baf3daf97d19bfde0de4 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 10 Oct 2023 17:56:32 -0500 Subject: [PATCH 14/34] fix: Removed landscape default group from GQL --- terraso_backend/apps/graphql/schema/landscapes.py | 1 - terraso_backend/apps/graphql/schema/schema.graphql | 1 - 2 files changed, 2 deletions(-) diff --git a/terraso_backend/apps/graphql/schema/landscapes.py b/terraso_backend/apps/graphql/schema/landscapes.py index da7fe39ca..9ebc40905 100644 --- a/terraso_backend/apps/graphql/schema/landscapes.py +++ b/terraso_backend/apps/graphql/schema/landscapes.py @@ -56,7 +56,6 @@ class LandscapeNode(DjangoObjectType, SharedResourcesMixin): id = graphene.ID(source="pk", required=True) area_types = graphene.List(graphene.String) - default_group = graphene.Field("apps.graphql.schema.groups.GroupNode") center_coordinates = graphene.Field(Point) class Meta: diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index 5e31bbb73..4541bbc9e 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -297,7 +297,6 @@ type LandscapeNode implements Node { id: ID! sharedResources(offset: Int, before: String, after: String, first: Int, last: Int, source_DataEntry_ResourceType_In: [String]): SharedResourceNodeConnection areaTypes: [String] - defaultGroup: GroupNode areaScalarHa: Float } From 3e63f964a10f76a3910506e362fc5294ffb2952d Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Wed, 11 Oct 2023 15:36:34 -0500 Subject: [PATCH 15/34] fix: Added enroll method to membership list node --- .../apps/collaboration/graphql/memberships.py | 1 + terraso_backend/apps/graphql/schema/schema.graphql | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/terraso_backend/apps/collaboration/graphql/memberships.py b/terraso_backend/apps/collaboration/graphql/memberships.py index 98011abe9..3e52305ca 100644 --- a/terraso_backend/apps/collaboration/graphql/memberships.py +++ b/terraso_backend/apps/collaboration/graphql/memberships.py @@ -53,6 +53,7 @@ class Meta: fields = ( "memberships", "membership_type", + "enroll_method", ) interfaces = (relay.Node,) connection_class = TerrasoConnection diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index 4541bbc9e..7665f4984 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -309,6 +309,7 @@ schema (one of the key benefits of GraphQL). scalar JSONString type CollaborationMembershipListNode implements Node { + enrollMethod: CollaborationMembershipListEnrollMethodChoices! membershipType: CollaborationMembershipListMembershipTypeChoices! memberships(offset: Int, before: String, after: String, first: Int, last: Int, user: ID, user_In: [ID], userRole: String, user_Email_Icontains: String, user_Email_In: [String], membershipStatus: CollaborationMembershipMembershipStatusChoices, user_Email_Not: String): CollaborationMembershipNodeConnection! id: ID! @@ -316,6 +317,18 @@ type CollaborationMembershipListNode implements Node { membershipsCount: Int } +"""An enumeration.""" +enum CollaborationMembershipListEnrollMethodChoices { + """Join""" + JOIN + + """Invite""" + INVITE + + """Both""" + BOTH +} + """An enumeration.""" enum CollaborationMembershipListMembershipTypeChoices { """Open""" @@ -700,6 +713,7 @@ type ProjectNode implements Node { } type ProjectMembershipListNode implements Node { + enrollMethod: CollaborationMembershipListEnrollMethodChoices! membershipType: CollaborationMembershipListMembershipTypeChoices! memberships(offset: Int, before: String, after: String, first: Int, last: Int, user: ID, user_In: [ID], userRole: String, user_Email_Icontains: String, user_Email_In: [String], membershipStatus: CollaborationMembershipMembershipStatusChoices, user_Email_Not: String): ProjectMembershipNodeConnection! id: ID! From a98f13b974b8fd6dfbce51eed891ab7370567443 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Wed, 11 Oct 2023 17:13:33 -0500 Subject: [PATCH 16/34] feat: Shared data files issue --- todo.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/todo.md b/todo.md index 43a6993b0..043c92699 100644 --- a/todo.md +++ b/todo.md @@ -1,5 +1,4 @@ # TODO -- migrate data to new landscape membership model - Prefetch for landscape directory -- Need logged in user to add landscape membership? -- Remove direct membership GQL mutations \ No newline at end of file +- Remove direct membership GQL mutations +- Shared data files linked through Groups \ No newline at end of file From 118a339a781f4433747363b7e453f273d024bd34 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Mon, 30 Oct 2023 10:37:03 -0500 Subject: [PATCH 17/34] fix: Renamed migration --- ...ape_membership_list.py => 0047_landscape_membership_list.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename terraso_backend/apps/core/migrations/{0046_landscape_membership_list.py => 0047_landscape_membership_list.py} (97%) diff --git a/terraso_backend/apps/core/migrations/0046_landscape_membership_list.py b/terraso_backend/apps/core/migrations/0047_landscape_membership_list.py similarity index 97% rename from terraso_backend/apps/core/migrations/0046_landscape_membership_list.py rename to terraso_backend/apps/core/migrations/0047_landscape_membership_list.py index fa65b638e..e13030592 100644 --- a/terraso_backend/apps/core/migrations/0046_landscape_membership_list.py +++ b/terraso_backend/apps/core/migrations/0047_landscape_membership_list.py @@ -53,7 +53,7 @@ def copy_memberships(apps, schema_editor): class Migration(migrations.Migration): dependencies = [ ("collaboration", "0005_change_collaborator_to_editor"), - ("core", "0045_taxonomyterms_ecosystems_renamed"), + ("core", "0046_shared_resource"), ] operations = [ From f9a9609207eee307fed840114840a425998f1f8b Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Mon, 30 Oct 2023 10:37:33 -0500 Subject: [PATCH 18/34] fix: Return landscape on membership delete --- terraso_backend/apps/core/models/landscapes.py | 2 +- terraso_backend/apps/graphql/schema/landscapes.py | 4 +++- terraso_backend/apps/graphql/schema/schema.graphql | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/terraso_backend/apps/core/models/landscapes.py b/terraso_backend/apps/core/models/landscapes.py index a78ec82d2..93e9dae81 100644 --- a/terraso_backend/apps/core/models/landscapes.py +++ b/terraso_backend/apps/core/models/landscapes.py @@ -15,8 +15,8 @@ import structlog from dirtyfields import DirtyFieldsMixin -from django.contrib.contenttypes.fields import GenericRelation from django.apps import apps +from django.contrib.contenttypes.fields import GenericRelation from django.db import models, transaction from apps.core import permission_rules as perm_rules diff --git a/terraso_backend/apps/graphql/schema/landscapes.py b/terraso_backend/apps/graphql/schema/landscapes.py index 9ebc40905..2058fe40d 100644 --- a/terraso_backend/apps/graphql/schema/landscapes.py +++ b/terraso_backend/apps/graphql/schema/landscapes.py @@ -443,6 +443,7 @@ def validate(context): class LandscapeMembershipDeleteMutation(BaseDeleteMutation): membership = graphene.Field(CollaborationMembershipNode) + landscape = graphene.Field(LandscapeNode) model_class = CollaborationMembership @@ -490,4 +491,5 @@ def mutate_and_get_payload(cls, root, info, **kwargs): model_name=CollaborationMembership.__name__, operation=MutationTypes.DELETE ) - return super().mutate_and_get_payload(root, info, **kwargs) + result = super().mutate_and_get_payload(root, info, **kwargs) + return cls(membership=result, landscape=landscape) diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index 7665f4984..806783d18 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -2499,6 +2499,7 @@ input LandscapeMembershipSaveMutationInput { type LandscapeMembershipDeleteMutationPayload { errors: GenericScalar membership: CollaborationMembershipNode + landscape: LandscapeNode clientMutationId: String } From 41de1b99c6f7e98638f524058ae777dcdcf75cfc Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Mon, 30 Oct 2023 14:48:02 -0500 Subject: [PATCH 19/34] fix: Shared data landscape membership fixes --- .../apps/collaboration/models/memberships.py | 2 +- .../apps/graphql/schema/data_entries.py | 6 +++--- .../apps/shared_data/permission_rules.py | 9 +++++++-- terraso_backend/tests/graphql/conftest.py | 18 +++++++++++++++++- .../mutations/test_shared_data_mutations.py | 16 +++++++++++----- .../tests/graphql/test_shared_data.py | 6 ++++-- 6 files changed, 43 insertions(+), 14 deletions(-) diff --git a/terraso_backend/apps/collaboration/models/memberships.py b/terraso_backend/apps/collaboration/models/memberships.py index 227bd43d2..1c17f5b8e 100644 --- a/terraso_backend/apps/collaboration/models/memberships.py +++ b/terraso_backend/apps/collaboration/models/memberships.py @@ -128,7 +128,7 @@ def approved_members(self): return User.objects.filter(id__in=approved_memberships_user_ids) def has_role(self, user: User, role: str) -> bool: - return self.memberships.by_role(role).filter(id=user.id).exists() + return self.memberships.by_role(role).filter(user=user).exists() def is_approved_member(self, user: User) -> bool: return self.approved_members.filter(id=user.id).exists() diff --git a/terraso_backend/apps/graphql/schema/data_entries.py b/terraso_backend/apps/graphql/schema/data_entries.py index a4add33b3..1307adc51 100644 --- a/terraso_backend/apps/graphql/schema/data_entries.py +++ b/terraso_backend/apps/graphql/schema/data_entries.py @@ -26,6 +26,7 @@ from graphene_django import DjangoObjectType from apps.core.gis.parsers import parse_file_to_geojson +from apps.collaboration.models import Membership as CollaborationMembership from apps.core.models import Group, Landscape, Membership from apps.graphql.exceptions import GraphQLNotAllowedException, GraphQLNotFoundException from apps.shared_data.models import DataEntry, VisualizationConfig @@ -104,9 +105,8 @@ def get_queryset(cls, queryset, info): ) user_landscape_ids = Subquery( Landscape.objects.filter( - associated_groups__group__memberships__user__id=user_pk, - associated_groups__group__memberships__membership_status=Membership.APPROVED, - associated_groups__is_default_landscape_group=True, + membership_list__memberships__user__id=user_pk, + membership_list__memberships__membership_status=CollaborationMembership.APPROVED, ).values("id") ) diff --git a/terraso_backend/apps/shared_data/permission_rules.py b/terraso_backend/apps/shared_data/permission_rules.py index cec25cacd..8e7c04b29 100644 --- a/terraso_backend/apps/shared_data/permission_rules.py +++ b/terraso_backend/apps/shared_data/permission_rules.py @@ -15,6 +15,7 @@ import rules +from apps.core import landscape_collaboration_roles from apps.core.models import Group, Landscape @@ -22,7 +23,11 @@ def is_target_manager(user, target): if isinstance(target, Group): return user.memberships.managers_only().filter(group=target).exists() if isinstance(target, Landscape): - return user.memberships.managers_only().filter(group=target.get_default_group()).exists() + return ( + target.membership_list.memberships.by_role(landscape_collaboration_roles.ROLE_MANAGER) + .filter(user=user) + .exists() + ) return False @@ -30,7 +35,7 @@ def is_target_member(user, target): if isinstance(target, Group): return user.memberships.approved_only().filter(group=target).exists() if isinstance(target, Landscape): - return user.memberships.approved_only().filter(group=target.get_default_group()).exists() + return target.membership_list.memberships.approved_only().filter(user=user).exists() return False diff --git a/terraso_backend/tests/graphql/conftest.py b/terraso_backend/tests/graphql/conftest.py index 121746a74..ae813edd7 100644 --- a/terraso_backend/tests/graphql/conftest.py +++ b/terraso_backend/tests/graphql/conftest.py @@ -16,6 +16,7 @@ from datetime import timedelta import pytest +from django.contrib.contenttypes.models import ContentType from django.utils import timezone from freezegun import freeze_time from graphene_django.utils.testing import graphql_query @@ -317,7 +318,7 @@ def group_data_entries(users, groups): @pytest.fixture -def landscape_data_entries(users, landscapes, landscape_groups): +def landscape_data_entries(users, landscapes): creator = users[0] creator_landscape = landscapes[0] resources = mixer.cycle(5).blend( @@ -330,6 +331,21 @@ def landscape_data_entries(users, landscapes, landscape_groups): return [resource.source for resource in resources] +@pytest.fixture +def landscape_data_entries_memberships(users, landscape_data_entries): + user = users[0] + for data_entry in landscape_data_entries: + shared_resource = data_entry.shared_resources.first() + if shared_resource.target_content_type == ContentType.objects.get( + app_label="core", model="landscape" + ): + shared_resource.target.membership_list.save_membership( + user_email=user.email, + user_role=landscape_collaboration_roles.ROLE_MEMBER, + membership_status=CollaborationMembership.APPROVED, + ) + + @pytest.fixture def data_entries(group_data_entries, landscape_data_entries): return group_data_entries + landscape_data_entries diff --git a/terraso_backend/tests/graphql/mutations/test_shared_data_mutations.py b/terraso_backend/tests/graphql/mutations/test_shared_data_mutations.py index 2716b718b..ceafa5a1d 100644 --- a/terraso_backend/tests/graphql/mutations/test_shared_data_mutations.py +++ b/terraso_backend/tests/graphql/mutations/test_shared_data_mutations.py @@ -15,7 +15,8 @@ import pytest -from apps.core.models import Membership +from apps.collaboration.models import Membership as CollaborationMembership +from apps.core.models import Group, Landscape, Membership from apps.shared_data.models import DataEntry pytestmark = pytest.mark.django_db @@ -205,19 +206,24 @@ def test_data_entry_delete_by_manager_works(client_query, data_entries, users, g def data_entry_by_not_manager_by_owner(request, users, landscape_data_entries, group_data_entries): owner = request.param - (data_entry, group) = ( + (data_entry, target) = ( (group_data_entries[0], group_data_entries[0].shared_resources.first().target) if owner == "group" else ( landscape_data_entries[0], - landscape_data_entries[0].shared_resources.first().target.get_default_group(), + landscape_data_entries[0].shared_resources.first().target, ) ) data_entry.created_by = users[2] data_entry.save() - group.add_manager(users[0]) - users[0].memberships.filter(group=group).update(membership_status=Membership.PENDING) + if isinstance(target, Group): + target.add_manager(users[0]) + users[0].memberships.filter(group=target).update(membership_status=Membership.PENDING) + if isinstance(target, Landscape): + target.membership_list.memberships.filter(user=users[0]).update( + membership_status=CollaborationMembership.PENDING + ) return data_entry diff --git a/terraso_backend/tests/graphql/test_shared_data.py b/terraso_backend/tests/graphql/test_shared_data.py index 73223ec5b..1687c2e41 100644 --- a/terraso_backend/tests/graphql/test_shared_data.py +++ b/terraso_backend/tests/graphql/test_shared_data.py @@ -29,7 +29,7 @@ pytestmark = pytest.mark.django_db -def test_data_entries_query(client_query, data_entries): +def test_data_entries_query(client_query, data_entries, landscape_data_entries_memberships): response = client_query( """ {dataEntries { @@ -69,7 +69,9 @@ def test_data_entry_get_one_by_id(client_query, data_entries): assert data_entry_result["name"] == data_entry.name -def test_data_entries_query_has_total_count(client_query, data_entries): +def test_data_entries_query_has_total_count( + client_query, data_entries, landscape_data_entries_memberships +): response = client_query( """ {dataEntries { From ccf22eaa45b2758d296133a48db6c889d302ce4c Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Mon, 30 Oct 2023 14:48:28 -0500 Subject: [PATCH 20/34] fix: Added landscape managers count --- terraso_backend/apps/core/permission_rules.py | 23 ++++++++++++ .../apps/graphql/schema/landscapes.py | 20 +++++++++- .../test_landscape_membership_mutations.py | 37 +++++++++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/terraso_backend/apps/core/permission_rules.py b/terraso_backend/apps/core/permission_rules.py index 5209f4327..d229dd135 100644 --- a/terraso_backend/apps/core/permission_rules.py +++ b/terraso_backend/apps/core/permission_rules.py @@ -15,6 +15,8 @@ import rules +from apps.core import landscape_collaboration_roles + @rules.predicate def allowed_to_change_group(user, group_id): @@ -98,6 +100,26 @@ def allowed_group_managers_count(user, membership_id): return True +@rules.predicate +def allowed_landscape_managers_count(user, obj): + membership = obj.get("membership") + landscape = obj.get("landscape") + + is_user_manager = landscape.membership_list.has_role( + user, landscape_collaboration_roles.ROLE_MANAGER + ) + managers_count = landscape.membership_list.memberships.by_role( + landscape_collaboration_roles.ROLE_MANAGER + ).count() + is_own_membership = user.collaboration_memberships.filter(pk=membership.id).exists() + + # User is the last manager and a Group cannot have no managers + if managers_count == 1 and is_user_manager and is_own_membership: + return False + + return True + + @rules.predicate def allowed_to_update_preferences(user, user_preferences): return user_preferences.user.id == user.id @@ -147,3 +169,4 @@ def allowed_to_delete_landscape_membership(user, obj): rules.add_rule("allowed_to_change_landscape", allowed_to_change_landscape) rules.add_rule("allowed_to_change_landscape_membership", allowed_to_change_landscape_membership) rules.add_rule("allowed_to_delete_landscape_membership", allowed_to_delete_landscape_membership) +rules.add_rule("allowed_landscape_managers_count", allowed_landscape_managers_count) diff --git a/terraso_backend/apps/graphql/schema/landscapes.py b/terraso_backend/apps/graphql/schema/landscapes.py index 2058fe40d..a2ca6d7da 100644 --- a/terraso_backend/apps/graphql/schema/landscapes.py +++ b/terraso_backend/apps/graphql/schema/landscapes.py @@ -491,5 +491,23 @@ def mutate_and_get_payload(cls, root, info, **kwargs): model_name=CollaborationMembership.__name__, operation=MutationTypes.DELETE ) + if not rules.test_rule( + "allowed_landscape_managers_count", + user, + { + "landscape": landscape, + "membership": membership, + }, + ): + logger.info( + "Attempt to update a Membership, but manager's count doesn't allow", + extra={"user_id": user.pk, "membership_id": membership_id}, + ) + raise GraphQLNotAllowedException( + model_name=Membership.__name__, + operation=MutationTypes.DELETE, + message="manager_count", + ) + result = super().mutate_and_get_payload(root, info, **kwargs) - return cls(membership=result, landscape=landscape) + return cls(membership=result.membership, landscape=landscape) diff --git a/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py b/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py index 81b666b72..3539dd603 100644 --- a/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py +++ b/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py @@ -450,3 +450,40 @@ def test_landscape_membership_delete_by_any_other_user( "delete_not_allowed" in response["data"]["deleteLandscapeMembership"]["errors"][0]["message"] ) + + +def test_landscape_membership_delete_by_last_manager(client_query, managed_landscapes, users): + landscape = managed_landscapes[0] + manager_membership = landscape.membership_list.memberships.by_role( + landscape_collaboration_roles.ROLE_MANAGER, + ).first() + + response = client_query( + """ + mutation deleteMembership($input: LandscapeMembershipDeleteMutationInput!){ + deleteLandscapeMembership(input: $input) { + membership { + user { + email + }, + } + errors + } + } + """, + variables={ + "input": { + "id": str(manager_membership.id), + "landscapeSlug": landscape.slug, + } + }, + ) + + response = response.json() + + assert "errors" in response["data"]["deleteLandscapeMembership"] + assert ( + "delete_not_allowed" + in response["data"]["deleteLandscapeMembership"]["errors"][0]["message"] + ) + assert "manager_count" in response["data"]["deleteLandscapeMembership"]["errors"][0]["message"] From e9a80b01c5a863e2af2ed6bc9d00942ec4b94a3e Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 31 Oct 2023 09:38:57 -0500 Subject: [PATCH 21/34] fix: User can join landscape --- .../apps/collaboration/models/memberships.py | 1 + terraso_backend/apps/core/permission_rules.py | 13 +++- .../test_landscape_membership_mutations.py | 70 ++++++++++++++++++- 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/terraso_backend/apps/collaboration/models/memberships.py b/terraso_backend/apps/collaboration/models/memberships.py index 1c17f5b8e..5446e8c6b 100644 --- a/terraso_backend/apps/collaboration/models/memberships.py +++ b/terraso_backend/apps/collaboration/models/memberships.py @@ -80,6 +80,7 @@ def save_membership( "membership_status": membership_status, "current_membership": membership, "user_exists": user_exists, + "user_email": user_email, } ): raise ValidationError("User cannot request membership") diff --git a/terraso_backend/apps/core/permission_rules.py b/terraso_backend/apps/core/permission_rules.py index d229dd135..1631b3849 100644 --- a/terraso_backend/apps/core/permission_rules.py +++ b/terraso_backend/apps/core/permission_rules.py @@ -145,12 +145,23 @@ def allowed_to_add_membership(user, group): @rules.predicate def allowed_to_change_landscape_membership(user, obj): landscape = obj.get("landscape") + user_role = obj.get("user_role") user_exists = obj.get("user_exists") + user_email = obj.get("user_email") + is_landscape_manager = user.is_landscape_manager(landscape.id) + own_membership = user_email == user.email if not user_exists: return False - return user.is_landscape_manager(landscape.id) + if ( + not is_landscape_manager + and own_membership + and user_role == landscape_collaboration_roles.ROLE_MEMBER + ): + return True + + return is_landscape_manager @rules.predicate diff --git a/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py b/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py index 3539dd603..69125053d 100644 --- a/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py +++ b/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py @@ -113,6 +113,7 @@ def test_landscape_membership_add_user_not_found(client_query, managed_landscape }, ) response = response.json() + print(response) assert "errors" in response["data"]["saveLandscapeMembership"] assert ( @@ -157,7 +158,7 @@ def test_landscape_membership_adding_duplicated_returns_existing_membership( def test_landscape_membership_add_manager(client_query, managed_landscapes, users): landscape = managed_landscapes[0] - user = users[0] + user = users[3] response = client_query( """ @@ -191,6 +192,73 @@ def test_landscape_membership_add_manager(client_query, managed_landscapes, user assert membership["membershipStatus"] == CollaborationMembership.APPROVED.upper() +def test_landscape_membership_add_manager_fail(client_query, managed_landscapes, users): + landscape = managed_landscapes[1] + user = users[3] + + response = client_query( + """ + mutation addMembership($input: LandscapeMembershipSaveMutationInput!){ + saveLandscapeMembership(input: $input) { + memberships { + id + } + errors + } + } + """, + variables={ + "input": { + "userEmails": [user.email], + "landscapeSlug": landscape.slug, + "userRole": landscape_collaboration_roles.ROLE_MANAGER, + } + }, + ) + response = response.json() + + assert "errors" in response["data"]["saveLandscapeMembership"] + assert ( + "update_not_allowed" in response["data"]["saveLandscapeMembership"]["errors"][0]["message"] + ) + + +def test_landscape_membership_join(client_query, managed_landscapes, users): + landscape = managed_landscapes[1] + user = users[0] + + response = client_query( + """ + mutation addMembership($input: LandscapeMembershipSaveMutationInput!){ + saveLandscapeMembership(input: $input) { + memberships { + id + userRole + membershipStatus + user { + email + } + } + } + } + """, + variables={ + "input": { + "userEmails": [user.email], + "landscapeSlug": landscape.slug, + "userRole": landscape_collaboration_roles.ROLE_MEMBER, + } + }, + ) + json_response = response.json() + membership = json_response["data"]["saveLandscapeMembership"]["memberships"][0] + + assert membership["id"] + assert membership["user"]["email"] == user.email + assert membership["userRole"] == landscape_collaboration_roles.ROLE_MEMBER + assert membership["membershipStatus"] == CollaborationMembership.APPROVED.upper() + + def test_landscape_membership_update(client_query, managed_landscapes, landscape_user_memberships): landscape = managed_landscapes[0] old_membership = landscape_user_memberships[0] From a17812838fe7e9fd80e0bd5ac08935fd7dedfdf0 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 31 Oct 2023 09:39:26 -0500 Subject: [PATCH 22/34] fix: Added membership email filter to landscapes query --- .../apps/graphql/schema/landscapes.py | 1 + .../apps/graphql/schema/schema.graphql | 2 +- .../tests/graphql/test_landscape.py | 35 +++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/terraso_backend/apps/graphql/schema/landscapes.py b/terraso_backend/apps/graphql/schema/landscapes.py index a2ca6d7da..3724cc374 100644 --- a/terraso_backend/apps/graphql/schema/landscapes.py +++ b/terraso_backend/apps/graphql/schema/landscapes.py @@ -66,6 +66,7 @@ class Meta: "slug": ["exact", "icontains"], "website": ["icontains"], "location": ["icontains"], + "membership_list__memberships__user__email": ["exact"], } fields = ( "name", diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index 806783d18..5ae8f3817 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -29,7 +29,7 @@ type Query { id: ID! ): GroupAssociationNode! groups(offset: Int, before: String, after: String, first: Int, last: Int, name: String, name_Icontains: String, name_Istartswith: String, slug: String, slug_Icontains: String, description_Icontains: String, memberships_Email: String, associatedLandscapes_IsDefaultLandscapeGroup: Boolean, associatedLandscapes_Isnull: Boolean, associatedLandscapes_IsPartnership: Boolean): GroupNodeConnection - landscapes(offset: Int, before: String, after: String, first: Int, last: Int, name_Icontains: String, description_Icontains: String, slug: String, slug_Icontains: String, website_Icontains: String, location_Icontains: String): LandscapeNodeConnection + landscapes(offset: Int, before: String, after: String, first: Int, last: Int, name_Icontains: String, description_Icontains: String, slug: String, slug_Icontains: String, website_Icontains: String, location_Icontains: String, membershipList_Memberships_User_Email: String): LandscapeNodeConnection users(offset: Int, before: String, after: String, first: Int, last: Int, email: String, email_Icontains: String, email_Iexact: String, firstName_Icontains: String, lastName_Icontains: String, project: String): UserNodeConnection landscapeGroups(offset: Int, before: String, after: String, first: Int, last: Int, landscape: ID, landscape_Slug_Icontains: String, group: ID, group_Slug_Icontains: String, isPartnership: Boolean): LandscapeGroupNodeConnection memberships(offset: Int, before: String, after: String, first: Int, last: Int, group: ID, group_In: [ID], group_Slug_Icontains: String, group_Slug_In: [String], user: ID, user_In: [ID], userRole: CoreMembershipUserRoleChoices, user_Email_Icontains: String, user_Email_In: [String], membershipStatus: CoreMembershipMembershipStatusChoices): MembershipNodeConnection diff --git a/terraso_backend/tests/graphql/test_landscape.py b/terraso_backend/tests/graphql/test_landscape.py index 862128488..7024acc72 100644 --- a/terraso_backend/tests/graphql/test_landscape.py +++ b/terraso_backend/tests/graphql/test_landscape.py @@ -201,3 +201,38 @@ def test_landscapes_query_with_membership_for_anonymous_user( memberships = membership_list["memberships"]["edges"] assert len(memberships) == 0 assert membership_list["membershipsCount"] == 0 + + +def test_landscapes_query_by_membership_email(client_query, landscape_user_memberships): + membership = landscape_user_memberships[0] + response = client_query( + """ + {landscapes(membershipList_Memberships_User_Email: "%s") { + edges { + node { + name + membershipList { + membershipsCount + memberships { + edges { + node { + user { + email + } + } + } + } + } + } + } + }} + """ + % membership.user.email + ) + + json_response = response.json() + + membership_list = json_response["data"]["landscapes"]["edges"][0]["node"]["membershipList"] + memberships = membership_list["memberships"]["edges"] + assert len(memberships) == 2 + assert membership_list["membershipsCount"] == 2 From 1fe84e2b9c75e4368dda53a8b3e7803515449ca0 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 31 Oct 2023 17:54:40 -0500 Subject: [PATCH 23/34] fix: Added membership list to landscape admin --- terraso_backend/apps/core/admin.py | 1 + 1 file changed, 1 insertion(+) diff --git a/terraso_backend/apps/core/admin.py b/terraso_backend/apps/core/admin.py index 0c6430b76..f84bf31dd 100644 --- a/terraso_backend/apps/core/admin.py +++ b/terraso_backend/apps/core/admin.py @@ -52,6 +52,7 @@ def get_queryset(self, request): @admin.register(Landscape) class LandscapeAdmin(admin.ModelAdmin): list_display = ("name", "slug", "location", "website", "created_at") + raw_id_fields = ("membership_list",) readonly_fields = ("default_group",) From 8deeda3728b8e19221df00e3b2ea9433ffda9f5e Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Wed, 1 Nov 2023 10:08:02 -0500 Subject: [PATCH 24/34] fix: Prefetch Landscape membership list data --- .../apps/collaboration/graphql/memberships.py | 6 ++++ .../apps/graphql/schema/landscapes.py | 30 ++++++------------- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/terraso_backend/apps/collaboration/graphql/memberships.py b/terraso_backend/apps/collaboration/graphql/memberships.py index 3e52305ca..0c8c08b1c 100644 --- a/terraso_backend/apps/collaboration/graphql/memberships.py +++ b/terraso_backend/apps/collaboration/graphql/memberships.py @@ -62,9 +62,15 @@ def resolve_account_membership(self, info): user = info.context.user if user.is_anonymous: return None + if hasattr(self, "account_memberships"): + if len(self.account_memberships) > 0: + return self.account_memberships[0] + return None return self.memberships.filter(user=user).first() def resolve_memberships_count(self, info): + if hasattr(self, "memberships_count"): + return self.memberships_count user = info.context.user if user.is_anonymous: return 0 diff --git a/terraso_backend/apps/graphql/schema/landscapes.py b/terraso_backend/apps/graphql/schema/landscapes.py index 3724cc374..bc3c2e9f0 100644 --- a/terraso_backend/apps/graphql/schema/landscapes.py +++ b/terraso_backend/apps/graphql/schema/landscapes.py @@ -24,6 +24,7 @@ from apps.collaboration.graphql import CollaborationMembershipNode from apps.collaboration.models import Membership as CollaborationMembership +from apps.collaboration.models import MembershipList from apps.core import landscape_collaboration_roles from apps.core.gis.utils import m2_to_hectares from apps.core.models import ( @@ -99,19 +100,19 @@ def get_queryset(cls, queryset, info): is_anonymous = info.context.user.is_anonymous try: - # Prefetch default landscape group, account membership and count of members - group_queryset = ( - Group.objects.prefetch_related( + # Prefetch account membership and count of members + membership_list_queryset = ( + MembershipList.objects.prefetch_related( Prefetch( "memberships", to_attr="account_memberships", - queryset=Membership.objects.filter( + queryset=CollaborationMembership.objects.filter( user=info.context.user, ), ), ) if not is_anonymous - else Group.objects.all() + else MembershipList.objects.all() ).annotate( memberships_count=Count( "memberships__user", @@ -120,20 +121,14 @@ def get_queryset(cls, queryset, info): & Q(memberships__membership_status=Membership.APPROVED), ) ) - landscape_group_queryset = LandscapeGroup.objects.prefetch_related( - Prefetch( - "group", - queryset=group_queryset, - ), - ).filter(is_default_landscape_group=True) + # Fetch all fields from Landscape, except for area_polygon result = ( queryset.defer("area_polygon") .prefetch_related( Prefetch( - "associated_groups", - to_attr="default_landscape_groups", - queryset=landscape_group_queryset, + "membership_list", + queryset=membership_list_queryset, ) ) .all() @@ -147,13 +142,6 @@ def resolve_area_scalar_ha(self, info): area = self.area_scalar_m2 return None if area is None else round(m2_to_hectares(area), 3) - def resolve_default_group(self, info): - if hasattr(self, "default_landscape_groups"): - if len(self.default_landscape_groups) > 0: - return self.default_landscape_groups[0].group - return None - return self.get_default_group() - class LandscapeDevelopmentStrategyNode(DjangoObjectType): id = graphene.ID(source="pk", required=True) From b9d550d3d363ddeeb4ad9a0227faf854a6c1715c Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Wed, 1 Nov 2023 10:11:12 -0500 Subject: [PATCH 25/34] fix: Landscape membership count visible for anonymous users --- terraso_backend/tests/graphql/test_landscape.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraso_backend/tests/graphql/test_landscape.py b/terraso_backend/tests/graphql/test_landscape.py index 7024acc72..602f989e2 100644 --- a/terraso_backend/tests/graphql/test_landscape.py +++ b/terraso_backend/tests/graphql/test_landscape.py @@ -200,7 +200,7 @@ def test_landscapes_query_with_membership_for_anonymous_user( membership_list = json_response["data"]["landscapes"]["edges"][0]["node"]["membershipList"] memberships = membership_list["memberships"]["edges"] assert len(memberships) == 0 - assert membership_list["membershipsCount"] == 0 + assert membership_list["membershipsCount"] == 2 def test_landscapes_query_by_membership_email(client_query, landscape_user_memberships): From 35b862b2cb12db7c643b2ddc61bf9d182044ec48 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Wed, 1 Nov 2023 11:34:45 -0500 Subject: [PATCH 26/34] fix: Removed more default group references --- terraso_backend/apps/core/admin.py | 27 ------------------- .../management/commands/loadsampledata.py | 10 +++---- .../apps/core/models/landscapes.py | 26 +++--------------- .../apps/graphql/schema/group_associations.py | 8 ++++++ terraso_backend/apps/graphql/schema/groups.py | 9 ++++--- .../apps/graphql/schema/schema.graphql | 2 +- .../graphql/schema/visualization_config.py | 6 ++--- .../tests/core/models/test_landscapes.py | 19 ------------- terraso_backend/tests/graphql/conftest.py | 4 ++- 9 files changed, 27 insertions(+), 84 deletions(-) diff --git a/terraso_backend/apps/core/admin.py b/terraso_backend/apps/core/admin.py index f84bf31dd..63ce772a2 100644 --- a/terraso_backend/apps/core/admin.py +++ b/terraso_backend/apps/core/admin.py @@ -14,8 +14,6 @@ # along with this program. If not, see https://www.gnu.org/licenses/. from django.contrib import admin -from django.urls import reverse -from django.utils.safestring import mark_safe from .models import ( Group, @@ -54,37 +52,12 @@ class LandscapeAdmin(admin.ModelAdmin): list_display = ("name", "slug", "location", "website", "created_at") raw_id_fields = ("membership_list",) - readonly_fields = ("default_group",) - - def default_group(self, obj): - group = obj.get_default_group() - url = reverse("admin:core_landscapedefaultgroup_change", args=[group.pk]) - return mark_safe(f'{group}') - - default_group.short_description = "Default Group" - class LandscapeDefaultGroup(Group): class Meta: proxy = True -@admin.register(LandscapeDefaultGroup) -class LandscapeDefaultGroupAdmin(admin.ModelAdmin): - list_display = ("name", "slug", "website", "created_at") - inlines = [MembershipInline] - - def get_queryset(self, request): - qs = super().get_queryset(request) - landscape_group_ids = [ - values[0] - for values in LandscapeGroup.objects.filter( - is_default_landscape_group=True - ).values_list("group__id") - ] - return qs.filter(id__in=landscape_group_ids) - - @admin.register(LandscapeGroup) class LandscapeGroupAdmin(admin.ModelAdmin): list_display = ("landscape", "group") diff --git a/terraso_backend/apps/core/management/commands/loadsampledata.py b/terraso_backend/apps/core/management/commands/loadsampledata.py index ed7fd82c5..bbab93417 100644 --- a/terraso_backend/apps/core/management/commands/loadsampledata.py +++ b/terraso_backend/apps/core/management/commands/loadsampledata.py @@ -13,6 +13,7 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see https://www.gnu.org/licenses/. +from apps.collaboration.models import MembershipList from apps.core.models import Group, Landscape, LandscapeGroup from ._base_airtable import BaseAirtableCommand @@ -47,13 +48,8 @@ def handle(self, *args, **kwargs): name=landscape_name, defaults=model_data ) - # Creates Landscape default group - default_group, _ = Group.objects.update_or_create(name=f"{landscape_name} Group") - landscape_group, _ = LandscapeGroup.objects.update_or_create( - landscape=landscape, - group=default_group, - defaults={"is_default_landscape_group": True}, - ) + # Creates Landscape membership list + membership_list, _ = MembershipList.objects.update_or_create(landscape=landscape) # Creates Partnership group partnership_name = landscape_data.get("Landscape Partnership Name") diff --git a/terraso_backend/apps/core/models/landscapes.py b/terraso_backend/apps/core/models/landscapes.py index 93e9dae81..d9c459811 100644 --- a/terraso_backend/apps/core/models/landscapes.py +++ b/terraso_backend/apps/core/models/landscapes.py @@ -137,35 +137,15 @@ def save(self, *args, **kwargs): super().save(*args, **kwargs) def delete(self, *args, **kwargs): - default_group = self.get_default_group() + membership_list = self.membership_list with transaction.atomic(): ret = super().delete(*args, **kwargs) - # default group should be deleted as well - if default_group is not None: - default_group.delete() + if membership_list is not None: + membership_list.delete() return ret - def get_default_group(self): - """ - A default Group in a Landscape is that Group where any - individual (associated or not with other Groups) is added when - associating directly with a Landscape. - """ - try: - # associated_groups is the related_name defined on - # LandscapeGroup relationship with Landscape. It returns a - # queryset of LandscapeGroup - landscape_group = self.associated_groups.get(is_default_landscape_group=True) - except LandscapeGroup.DoesNotExist: - logger.error( - "Landscape has no default group, but it must have", extra={"landscape_id": self.pk} - ) - return None - - return landscape_group.group - def __str__(self): return self.name diff --git a/terraso_backend/apps/graphql/schema/group_associations.py b/terraso_backend/apps/graphql/schema/group_associations.py index 5d00a5af0..444202973 100644 --- a/terraso_backend/apps/graphql/schema/group_associations.py +++ b/terraso_backend/apps/graphql/schema/group_associations.py @@ -16,6 +16,7 @@ import graphene import structlog from django.core.exceptions import ValidationError +from django.db.models import Q from graphene import relay from graphene_django import DjangoObjectType @@ -47,6 +48,13 @@ class Meta: interfaces = (relay.Node,) connection_class = TerrasoConnection + @classmethod + def get_queryset(cls, queryset, info): + return queryset.exclude( + Q(parent_group__associated_landscapes__is_default_landscape_group=True) + | Q(child_group__associated_landscapes__is_default_landscape_group=True) + ) + class GroupAssociationAddMutation(BaseAuthenticatedMutation): group_association = graphene.Field(GroupAssociationNode) diff --git a/terraso_backend/apps/graphql/schema/groups.py b/terraso_backend/apps/graphql/schema/groups.py index e9c23f22c..e87822900 100644 --- a/terraso_backend/apps/graphql/schema/groups.py +++ b/terraso_backend/apps/graphql/schema/groups.py @@ -31,9 +31,6 @@ class GroupFilterSet(django_filters.FilterSet): 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" ) @@ -84,6 +81,12 @@ class Meta: interfaces = (relay.Node,) connection_class = TerrasoConnection + @classmethod + def get_queryset(cls, queryset, info): + return queryset.exclude( + associated_landscapes__is_default_landscape_group=True, + ) + def resolve_account_membership(self, info): user = info.context.user if user.is_anonymous: diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index 5ae8f3817..8377c42f4 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -28,7 +28,7 @@ type Query { """The ID of the object""" id: ID! ): GroupAssociationNode! - groups(offset: Int, before: String, after: String, first: Int, last: Int, name: String, name_Icontains: String, name_Istartswith: String, slug: String, slug_Icontains: String, description_Icontains: String, memberships_Email: String, associatedLandscapes_IsDefaultLandscapeGroup: Boolean, associatedLandscapes_Isnull: Boolean, associatedLandscapes_IsPartnership: Boolean): GroupNodeConnection + groups(offset: Int, before: String, after: String, first: Int, last: Int, name: String, name_Icontains: String, name_Istartswith: String, slug: String, slug_Icontains: String, description_Icontains: String, memberships_Email: String, associatedLandscapes_Isnull: Boolean, associatedLandscapes_IsPartnership: Boolean): GroupNodeConnection landscapes(offset: Int, before: String, after: String, first: Int, last: Int, name_Icontains: String, description_Icontains: String, slug: String, slug_Icontains: String, website_Icontains: String, location_Icontains: String, membershipList_Memberships_User_Email: String): LandscapeNodeConnection users(offset: Int, before: String, after: String, first: Int, last: Int, email: String, email_Icontains: String, email_Iexact: String, firstName_Icontains: String, lastName_Icontains: String, project: String): UserNodeConnection landscapeGroups(offset: Int, before: String, after: String, first: Int, last: Int, landscape: ID, landscape_Slug_Icontains: String, group: ID, group_Slug_Icontains: String, isPartnership: Boolean): LandscapeGroupNodeConnection diff --git a/terraso_backend/apps/graphql/schema/visualization_config.py b/terraso_backend/apps/graphql/schema/visualization_config.py index d5d37d9d0..35bb3c6b2 100644 --- a/terraso_backend/apps/graphql/schema/visualization_config.py +++ b/terraso_backend/apps/graphql/schema/visualization_config.py @@ -22,6 +22,7 @@ from graphene import relay from graphene_django import DjangoObjectType +from apps.collaboration.models import Membership as CollaborationMembership from apps.core.gis.mapbox import get_publish_status from apps.core.models import Group, Landscape, Membership from apps.graphql.exceptions import GraphQLNotAllowedException @@ -116,9 +117,8 @@ def get_queryset(cls, queryset, info): ) user_landscape_ids = Subquery( Landscape.objects.filter( - associated_groups__group__memberships__user__id=user_pk, - associated_groups__group__memberships__membership_status=Membership.APPROVED, - associated_groups__is_default_landscape_group=True, + membership_list__memberships__user__id=user_pk, + membership_list__memberships__membership_status=CollaborationMembership.APPROVED, ).values("id") ) return queryset.filter( diff --git a/terraso_backend/tests/core/models/test_landscapes.py b/terraso_backend/tests/core/models/test_landscapes.py index 952d67616..b5761df00 100644 --- a/terraso_backend/tests/core/models/test_landscapes.py +++ b/terraso_backend/tests/core/models/test_landscapes.py @@ -95,25 +95,6 @@ def test_landscape_groups_creation_explicitly(): assert group.associated_landscapes.count() == 1 -def test_landscape_get_default_group(): - landscape = mixer.blend(Landscape) - groups = mixer.cycle(3).blend(Group) - default_group = groups.pop() - - LandscapeGroup.objects.create( - landscape=landscape, group=default_group, is_default_landscape_group=True - ) - landscape.groups.add(*groups) - - assert landscape.get_default_group() == default_group - - -def test_landscape_get_default_group_without_group_returns_none(): - landscape = mixer.blend(Landscape) - - assert landscape.get_default_group() is None - - def test_landscape_creator_becomes_manager(): user = mixer.blend(User) landscape = mixer.blend(Landscape, created_by=user) diff --git a/terraso_backend/tests/graphql/conftest.py b/terraso_backend/tests/graphql/conftest.py index ae813edd7..e9288e00b 100644 --- a/terraso_backend/tests/graphql/conftest.py +++ b/terraso_backend/tests/graphql/conftest.py @@ -249,7 +249,9 @@ def landscape_common_group(landscapes, groups): group = groups[1] landscape = landscapes[0] - common_group = mixer.blend(LandscapeGroup, landscape=landscape, group=group) + common_group = mixer.blend( + LandscapeGroup, landscape=landscape, group=group, is_default_landscape_group=False + ) return common_group From dfbf5e46d290a30026bc1fa2d778931c254e47d8 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Wed, 1 Nov 2023 12:11:10 -0500 Subject: [PATCH 27/34] fix: Added distinct for landscapes query --- terraso_backend/apps/graphql/schema/landscapes.py | 1 + 1 file changed, 1 insertion(+) diff --git a/terraso_backend/apps/graphql/schema/landscapes.py b/terraso_backend/apps/graphql/schema/landscapes.py index bc3c2e9f0..e2fdfbbaf 100644 --- a/terraso_backend/apps/graphql/schema/landscapes.py +++ b/terraso_backend/apps/graphql/schema/landscapes.py @@ -132,6 +132,7 @@ def get_queryset(cls, queryset, info): ) ) .all() + .distinct() ) except Exception as e: logger.exception("Error prefetching Landscape data", error=e) From ae543437c986964af224d75b8ec74242587d7647 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 7 Nov 2023 12:53:59 -0500 Subject: [PATCH 28/34] Update terraso_backend/apps/graphql/schema/landscapes.py Co-authored-by: Paul Schreiber --- terraso_backend/apps/graphql/schema/landscapes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraso_backend/apps/graphql/schema/landscapes.py b/terraso_backend/apps/graphql/schema/landscapes.py index e2fdfbbaf..27779186e 100644 --- a/terraso_backend/apps/graphql/schema/landscapes.py +++ b/terraso_backend/apps/graphql/schema/landscapes.py @@ -490,7 +490,7 @@ def mutate_and_get_payload(cls, root, info, **kwargs): }, ): logger.info( - "Attempt to update a Membership, but manager's count doesn't allow", + "Attempt to update a Membership, but cannot remove last manager", extra={"user_id": user.pk, "membership_id": membership_id}, ) raise GraphQLNotAllowedException( From 63405d3a6bfd49b0364baf197a08c49fdca912ec Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 7 Nov 2023 12:54:28 -0500 Subject: [PATCH 29/34] Update terraso_backend/apps/graphql/schema/landscapes.py Co-authored-by: Paul Schreiber --- terraso_backend/apps/graphql/schema/landscapes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraso_backend/apps/graphql/schema/landscapes.py b/terraso_backend/apps/graphql/schema/landscapes.py index 27779186e..7051d1e8e 100644 --- a/terraso_backend/apps/graphql/schema/landscapes.py +++ b/terraso_backend/apps/graphql/schema/landscapes.py @@ -460,7 +460,7 @@ def mutate_and_get_payload(cls, root, info, **kwargs): membership = landscape.membership_list.memberships.get(id=membership_id) except CollaborationMembership.DoesNotExist: logger.error( - "Attempt to delete Landscape Membership, but it was not found", + "Attempt to delete Landscape Membership, but membership was not found", extra={"membership_id": membership_id}, ) raise GraphQLNotFoundException(model_name=CollaborationMembership.__name__) From f6fe2bd72e8a9bb25d2d1baf0c940b246ac2e022 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 7 Nov 2023 12:54:34 -0500 Subject: [PATCH 30/34] Update terraso_backend/apps/graphql/schema/landscapes.py Co-authored-by: Paul Schreiber --- terraso_backend/apps/graphql/schema/landscapes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraso_backend/apps/graphql/schema/landscapes.py b/terraso_backend/apps/graphql/schema/landscapes.py index 7051d1e8e..195589c45 100644 --- a/terraso_backend/apps/graphql/schema/landscapes.py +++ b/terraso_backend/apps/graphql/schema/landscapes.py @@ -474,7 +474,7 @@ def mutate_and_get_payload(cls, root, info, **kwargs): }, ): logger.info( - "Attempt to delete Landscape Memberships, but user has no permission", + "Attempt to delete Landscape Memberships, but user lacks permission", extra={"user_id": user.pk, "membership_id": membership_id}, ) raise GraphQLNotAllowedException( From e4aecc04e60a6acd6118813fa2b5946816c69bc0 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 7 Nov 2023 12:54:39 -0500 Subject: [PATCH 31/34] Update terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py Co-authored-by: Paul Schreiber --- .../graphql/mutations/test_landscape_membership_mutations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py b/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py index 69125053d..f2427aa9b 100644 --- a/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py +++ b/terraso_backend/tests/graphql/mutations/test_landscape_membership_mutations.py @@ -1,4 +1,4 @@ -# Copyright © 2021-2023 Technology Matters +# Copyright © 2023 Technology Matters # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU Affero General Public License as published From 64de363c5d7ba97126669566462219edf839a333 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 7 Nov 2023 12:55:06 -0500 Subject: [PATCH 32/34] Update terraso_backend/apps/collaboration/graphql/memberships.py Co-authored-by: Paul Schreiber --- terraso_backend/apps/collaboration/graphql/memberships.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraso_backend/apps/collaboration/graphql/memberships.py b/terraso_backend/apps/collaboration/graphql/memberships.py index 0c8c08b1c..362f7f0ff 100644 --- a/terraso_backend/apps/collaboration/graphql/memberships.py +++ b/terraso_backend/apps/collaboration/graphql/memberships.py @@ -63,7 +63,7 @@ def resolve_account_membership(self, info): if user.is_anonymous: return None if hasattr(self, "account_memberships"): - if len(self.account_memberships) > 0: + if len(self.account_memberships): return self.account_memberships[0] return None return self.memberships.filter(user=user).first() From 52a15aa0a60486abb7235f20e9b51541f2d1dfe7 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 7 Nov 2023 13:39:08 -0500 Subject: [PATCH 33/34] fix: Fixed imports --- terraso_backend/apps/core/models/landscapes.py | 2 +- terraso_backend/apps/core/models/users.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/terraso_backend/apps/core/models/landscapes.py b/terraso_backend/apps/core/models/landscapes.py index d9c459811..1bb5d266b 100644 --- a/terraso_backend/apps/core/models/landscapes.py +++ b/terraso_backend/apps/core/models/landscapes.py @@ -24,9 +24,9 @@ calculate_geojson_centroid, calculate_geojson_feature_area, ) +from apps.core.landscape_collaboration_roles import ROLE_MANAGER from apps.core.models.taxonomy_terms import TaxonomyTerm -from ..landscape_collaboration_roles import ROLE_MANAGER from .commons import BaseModel, SlugModel, validate_name from .groups import Group from .shared_resources import SharedResource diff --git a/terraso_backend/apps/core/models/users.py b/terraso_backend/apps/core/models/users.py index b782467b6..960605e84 100644 --- a/terraso_backend/apps/core/models/users.py +++ b/terraso_backend/apps/core/models/users.py @@ -21,7 +21,7 @@ from django.utils.translation import gettext_lazy as _ from safedelete.models import SOFT_DELETE_CASCADE, SafeDeleteManager, SafeDeleteModel -from .. import landscape_collaboration_roles +from apps.core import landscape_collaboration_roles NOTIFICATION_KEY_GROUP = "group_notifications" NOTIFICATION_KEY_STORY_MAP = "story_map_notifications" From e939b427d67495f1eae76928593506920aa7ef66 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Fri, 10 Nov 2023 11:52:04 -0500 Subject: [PATCH 34/34] fix: Linter fix --- terraso_backend/apps/graphql/schema/data_entries.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraso_backend/apps/graphql/schema/data_entries.py b/terraso_backend/apps/graphql/schema/data_entries.py index 1307adc51..b08e7fd5d 100644 --- a/terraso_backend/apps/graphql/schema/data_entries.py +++ b/terraso_backend/apps/graphql/schema/data_entries.py @@ -25,8 +25,8 @@ from graphene import relay from graphene_django import DjangoObjectType -from apps.core.gis.parsers import parse_file_to_geojson from apps.collaboration.models import Membership as CollaborationMembership +from apps.core.gis.parsers import parse_file_to_geojson from apps.core.models import Group, Landscape, Membership from apps.graphql.exceptions import GraphQLNotAllowedException, GraphQLNotFoundException from apps.shared_data.models import DataEntry, VisualizationConfig