From 4170565525a1b85462685d4e2c861a3cfeec7fba Mon Sep 17 00:00:00 2001 From: David Code Howard Date: Wed, 27 Sep 2023 17:32:29 -0400 Subject: [PATCH] feat: Use MembershipList with Projects (#793) * feat: Switch projects to use membership list; skip failing tests * feat: Create mutation to update membership list of project * feat: Restrict membership roles during creation * feat: Add mutation for removing users from project * test: Test permissions for deleting memberships * test: Test user not member of project cannot update * test: Test user can remove themself from project * feat: Add mutation for updating membership role * test: Add test for user not part of project updating role * test: Reactivate skipped project tests * feat: Move projects graphql into projects app * refactor: Define and use project management collaboration roles * chore: Make migration for project membership list * fix: Enum types for ProjectMemberships, PR fixups * feat: Use mixins to allow customizing MembershipNodes * feat: Use enum for project update mutations * fix: Restore missing membership_lists filter This can be done be reusing the filterset class with DjangoFilterConnectionField. It would be worthwhile checking if this work can be encapsulated in a resuable function, as there is now a lot of custom code whose function is not obvious. * fix: Fix schema errors with ProjectMembership The ProjectMembershipList was being a bunch of places where we did not want it to be used, like Story Maps! I tried creating using some Django proxy inheritance and that seems to have helped Graphene figure things out. * fix: Change Project model to point to proxy * fix: Fix malfunctioning ProjectMembershipListNode Somewhat confusing. * fix: Fix project filter to correct memberships Previously it was just returning all memberships for every project! * chore: Reorder migrations --- .../apps/collaboration/graphql/memberships.py | 38 +- .../apps/graphql/schema/__init__.py | 22 +- terraso_backend/apps/graphql/schema/groups.py | 4 - .../apps/graphql/schema/schema.graphql | 87 +++- terraso_backend/apps/graphql/schema/sites.py | 2 +- terraso_backend/apps/graphql/signals.py | 1 + .../apps/project_management/admin.py | 2 +- .../project_management/collaboration_roles.py | 18 + .../project_management/graphql/projects.py | 438 ++++++++++++++++++ ...e_project_group_project_membership_list.py | 31 ++ .../migrations/0016_auto_20230915_1527.py | 57 +++ .../migrations/0017_auto_20230915_1527.py | 26 ++ ...mbership_projectmembershiplist_and_more.py | 57 +++ ...y => 0019_project_seen_by_site_seen_by.py} | 2 +- .../project_management/models/__init__.py | 4 +- .../project_management/models/projects.py | 83 +++- .../apps/project_management/models/sites.py | 4 +- .../project_management/permission_rules.py | 44 ++ .../apps/project_management/signals.py | 17 +- .../apps/story_map/collaboration_roles.py | 1 + terraso_backend/tests/conftest.py | 16 +- .../mutations/test_membership_mutations.py | 41 +- .../tests/graphql/mutations/test_projects.py | 339 +++++++++----- .../tests/graphql/mutations/test_sites.py | 5 +- .../tests/graphql/test_projects_query.py | 4 +- .../tests/graphql/test_sites_query.py | 3 - 26 files changed, 1121 insertions(+), 225 deletions(-) create mode 100644 terraso_backend/apps/project_management/collaboration_roles.py create mode 100644 terraso_backend/apps/project_management/graphql/projects.py create mode 100644 terraso_backend/apps/project_management/migrations/0015_remove_project_group_project_membership_list.py create mode 100644 terraso_backend/apps/project_management/migrations/0016_auto_20230915_1527.py create mode 100644 terraso_backend/apps/project_management/migrations/0017_auto_20230915_1527.py create mode 100644 terraso_backend/apps/project_management/migrations/0018_projectmembership_projectmembershiplist_and_more.py rename terraso_backend/apps/project_management/migrations/{0015_project_seen_by_site_seen_by.py => 0019_project_seen_by_site_seen_by.py} (88%) diff --git a/terraso_backend/apps/collaboration/graphql/memberships.py b/terraso_backend/apps/collaboration/graphql/memberships.py index 36c98f85b..635a45c56 100644 --- a/terraso_backend/apps/collaboration/graphql/memberships.py +++ b/terraso_backend/apps/collaboration/graphql/memberships.py @@ -19,14 +19,34 @@ from graphene import relay from graphene_django import DjangoObjectType -from apps.graphql.schema.commons import TerrasoConnection - from ..models import Membership, MembershipList +# from apps.graphql.schema.commons import TerrasoConnection + + logger = structlog.get_logger(__name__) -class CollaborationMembershipListNode(DjangoObjectType): +# TODO: trying to import this from apps.graphql.schema.commons causes a circular import +# Created an issue to move the module to apps.graphql.commons, as that seems simplest +# https://github.com/techmatters/terraso-backend/issues/820 +class TerrasoConnection(graphene.Connection): + class Meta: + abstract = True + + total_count = graphene.Int(required=True) + + def resolve_total_count(self, info, **kwargs): + queryset = self.iterable + return queryset.count() + + @classmethod + def __init_subclass_with_meta__(cls, **options): + options["strict_types"] = options.pop("strict_types", True) + super().__init_subclass_with_meta__(**options) + + +class MembershipListNodeMixin: id = graphene.ID(source="pk", required=True) account_membership = graphene.Field("apps.collaboration.graphql.CollaborationMembershipNode") memberships_count = graphene.Int() @@ -53,6 +73,11 @@ def resolve_memberships_count(self, info): return self.memberships.approved_only().count() +class CollaborationMembershipListNode(MembershipListNodeMixin, DjangoObjectType): + class Meta(MembershipListNodeMixin.Meta): + pass + + class CollaborationMembershipFilterSet(django_filters.FilterSet): user__email__not = django_filters.CharFilter(method="filter_user_email_not") @@ -69,7 +94,7 @@ def filter_user_email_not(self, queryset, name, value): return queryset.exclude(user__email=value) -class CollaborationMembershipNode(DjangoObjectType): +class MembershipNodeMixin: id = graphene.ID(source="pk", required=True) class Meta: @@ -86,3 +111,8 @@ def get_queryset(cls, queryset, info): return queryset.none() return queryset + + +class CollaborationMembershipNode(MembershipNodeMixin, DjangoObjectType): + class Meta(MembershipNodeMixin.Meta): + pass diff --git a/terraso_backend/apps/graphql/schema/__init__.py b/terraso_backend/apps/graphql/schema/__init__.py index 68a0622c2..c4334deff 100644 --- a/terraso_backend/apps/graphql/schema/__init__.py +++ b/terraso_backend/apps/graphql/schema/__init__.py @@ -16,6 +16,17 @@ import graphene from graphene_django.filter import DjangoFilterConnectionField +from apps.project_management.graphql.projects import ( + ProjectAddMutation, + ProjectAddUserMutation, + ProjectArchiveMutation, + ProjectDeleteMutation, + ProjectDeleteUserMutation, + ProjectMarkSeenMutation, + ProjectNode, + ProjectUpdateMutation, + ProjectUpdateUserRoleMutation, +) from apps.soil_id.graphql.soil_data import ( DepthDependentSoilDataUpdateMutation, SoilDataUpdateMutation, @@ -57,14 +68,6 @@ MembershipNode, MembershipUpdateMutation, ) -from .projects import ( - ProjectAddMutation, - ProjectArchiveMutation, - ProjectDeleteMutation, - ProjectMarkSeenMutation, - ProjectNode, - ProjectUpdateMutation, -) from .sites import ( SiteAddMutation, SiteDeleteMutation, @@ -169,6 +172,9 @@ class Mutations(graphene.ObjectType): update_project = ProjectUpdateMutation.Field() archive_project = ProjectArchiveMutation.Field() delete_project = ProjectDeleteMutation.Field() + add_user_to_project = ProjectAddUserMutation.Field() + delete_user_from_project = ProjectDeleteUserMutation.Field() + update_user_role_in_project = ProjectUpdateUserRoleMutation.Field() mark_project_seen = ProjectMarkSeenMutation.Field() update_soil_data = SoilDataUpdateMutation.Field() update_depth_dependent_soil_data = DepthDependentSoilDataUpdateMutation.Field() diff --git a/terraso_backend/apps/graphql/schema/groups.py b/terraso_backend/apps/graphql/schema/groups.py index 38320e5e4..bcbb5d748 100644 --- a/terraso_backend/apps/graphql/schema/groups.py +++ b/terraso_backend/apps/graphql/schema/groups.py @@ -64,10 +64,6 @@ class GroupNode(DjangoObjectType): account_membership = graphene.Field("apps.graphql.schema.memberships.MembershipNode") memberships_count = graphene.Int() - @classmethod - def get_queryset(cls, queryset, info): - return queryset.filter(project__isnull=True) - class Meta: model = Group fields = ( diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index c459bf720..f23741234 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -636,7 +636,7 @@ type ProjectNode implements Node { updatedAt: DateTime! name: String! description: String! - group: GroupNode! + membershipList: ProjectMembershipListNode! privacy: ProjectManagementProjectPrivacyChoices! archived: Boolean! siteSet( @@ -658,6 +658,47 @@ type ProjectNode implements Node { seen: Boolean! } +type ProjectMembershipListNode implements Node { + membershipType: CollaborationMembershipListMembershipTypeChoices! + id: ID! + accountMembership: CollaborationMembershipNode + membershipsCount: Int + 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! +} + +type ProjectMembershipNodeConnection { + """Pagination data for this connection.""" + pageInfo: PageInfo! + + """Contains the nodes in this connection.""" + edges: [ProjectMembershipNodeEdge!]! + totalCount: Int! +} + +"""A Relay edge containing a `ProjectMembershipNode` and its cursor.""" +type ProjectMembershipNodeEdge { + """The item at the end of the edge""" + node: ProjectMembershipNode! + + """A cursor for use in pagination""" + cursor: String! +} + +type ProjectMembershipNode implements Node { + membershipList: CollaborationMembershipListNode! + user: UserNode + userRole: UserRole! + membershipStatus: CollaborationMembershipMembershipStatusChoices! + pendingEmail: String + id: ID! +} + +enum UserRole { + viewer + contributor + manager +} + """An enumeration.""" enum ProjectManagementProjectPrivacyChoices { """Private""" @@ -1303,6 +1344,9 @@ type Mutations { updateProject(input: ProjectUpdateMutationInput!): ProjectUpdateMutationPayload! archiveProject(input: ProjectArchiveMutationInput!): ProjectArchiveMutationPayload! deleteProject(input: ProjectDeleteMutationInput!): ProjectDeleteMutationPayload! + addUserToProject(input: ProjectAddUserMutationInput!): ProjectAddUserMutationPayload! + deleteUserFromProject(input: ProjectDeleteUserMutationInput!): ProjectDeleteUserMutationPayload! + updateUserRoleInProject(input: ProjectUpdateUserRoleMutationInput!): ProjectUpdateUserRoleMutationPayload! markProjectSeen(input: ProjectMarkSeenMutationInput!): ProjectMarkSeenMutationPayload! updateSoilData(input: SoilDataUpdateMutationInput!): SoilDataUpdateMutationPayload! updateDepthDependentSoilData(input: DepthDependentSoilDataUpdateMutationInput!): DepthDependentSoilDataUpdateMutationPayload! @@ -1814,6 +1858,47 @@ input ProjectDeleteMutationInput { clientMutationId: String } +type ProjectAddUserMutationPayload { + errors: GenericScalar + project: ProjectNode! + membership: CollaborationMembershipNode! + clientMutationId: String +} + +input ProjectAddUserMutationInput { + projectId: ID! + userId: ID! + role: UserRole! + clientMutationId: String +} + +type ProjectDeleteUserMutationPayload { + errors: GenericScalar + project: ProjectNode! + membership: CollaborationMembershipNode! + clientMutationId: String +} + +input ProjectDeleteUserMutationInput { + projectId: ID! + userId: ID! + clientMutationId: String +} + +type ProjectUpdateUserRoleMutationPayload { + errors: GenericScalar + project: ProjectNode! + membership: CollaborationMembershipNode! + clientMutationId: String +} + +input ProjectUpdateUserRoleMutationInput { + projectId: ID! + userId: ID! + newRole: UserRole! + clientMutationId: String +} + type ProjectMarkSeenMutationPayload { errors: GenericScalar project: ProjectNode! diff --git a/terraso_backend/apps/graphql/schema/sites.py b/terraso_backend/apps/graphql/schema/sites.py index 281f005d2..8da7dcb88 100644 --- a/terraso_backend/apps/graphql/schema/sites.py +++ b/terraso_backend/apps/graphql/schema/sites.py @@ -37,7 +37,7 @@ class SiteFilter(django_filters.FilterSet): project = TypedFilter() owner = TypedFilter() - project__member = TypedFilter(field_name="project__group__memberships__user") + project__member = TypedFilter(field_name="project__membership_list__memberships__user") class Meta: model = Site diff --git a/terraso_backend/apps/graphql/signals.py b/terraso_backend/apps/graphql/signals.py index c003f1a64..48f2c7bda 100644 --- a/terraso_backend/apps/graphql/signals.py +++ b/terraso_backend/apps/graphql/signals.py @@ -17,3 +17,4 @@ membership_added_signal = Signal() membership_updated_signal = Signal() +membership_deleted_signal = Signal() diff --git a/terraso_backend/apps/project_management/admin.py b/terraso_backend/apps/project_management/admin.py index 4ac6dbd11..1526e1330 100644 --- a/terraso_backend/apps/project_management/admin.py +++ b/terraso_backend/apps/project_management/admin.py @@ -25,4 +25,4 @@ @admin.register(Project) class ProjectAdmin(admin.ModelAdmin): - readonly_fields = ("group", "settings") + readonly_fields = ("membership_list", "settings") diff --git a/terraso_backend/apps/project_management/collaboration_roles.py b/terraso_backend/apps/project_management/collaboration_roles.py new file mode 100644 index 000000000..180928ecf --- /dev/null +++ b/terraso_backend/apps/project_management/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_CONTRIBUTOR = "contributor" +ROLE_VIEWER = "viewer" diff --git a/terraso_backend/apps/project_management/graphql/projects.py b/terraso_backend/apps/project_management/graphql/projects.py new file mode 100644 index 000000000..d190af631 --- /dev/null +++ b/terraso_backend/apps/project_management/graphql/projects.py @@ -0,0 +1,438 @@ +# 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/. + +from datetime import datetime + +import graphene +import rules +from django.core.exceptions import ValidationError +from django.db import transaction +from django_filters import CharFilter, FilterSet +from graphene import relay +from graphene_django import DjangoObjectType +from graphene_django.filter import DjangoFilterConnectionField, TypedFilter + +from apps.audit_logs import api as log_api +from apps.collaboration.graphql.memberships import ( + CollaborationMembershipNode as MembershipNode, +) +from apps.collaboration.graphql.memberships import ( + MembershipListNodeMixin, + MembershipNodeMixin, +) +from apps.collaboration.models import Membership +from apps.core.models import User +from apps.graphql.exceptions import GraphQLValidationException +from apps.graphql.schema.commons import ( + BaseAuthenticatedMutation, + BaseDeleteMutation, + BaseMutation, + BaseWriteMutation, + TerrasoConnection, +) +from apps.graphql.schema.constants import MutationTypes +from apps.graphql.signals import ( + membership_added_signal, + membership_deleted_signal, + membership_updated_signal, +) +from apps.project_management import collaboration_roles +from apps.project_management.models import ( + Project, + ProjectMembership, + ProjectMembershipList, +) +from apps.project_management.models.sites import Site + + +class UserRole(graphene.Enum): + viewer = collaboration_roles.ROLE_VIEWER + contributor = collaboration_roles.ROLE_CONTRIBUTOR + manager = collaboration_roles.ROLE_MANAGER + + +class ProjectMembershipNode(DjangoObjectType, MembershipNodeMixin): + class Meta(MembershipNodeMixin.Meta): + model = ProjectMembership + + user_role = graphene.Field(UserRole, required=True) + + def resolve_user_role(self, info): + match self.user_role: + case "viewer": + return UserRole.viewer + case "constributor": + return UserRole.contributor + case "manager": + return UserRole.manager + case _: + raise Exception(f"Unexpected user role: {self.user_role}") + + +class ProjectMembershipFilterSet(FilterSet): + user__email__not = CharFilter(method="filter_user_email_not") + + class Meta: + model = ProjectMembership + fields = { + "user": ["exact", "in"], + "user_role": ["exact"], + "user__email": ["icontains", "in"], + "membership_status": ["exact"], + } + + def filter_user_email_not(self, queryset, name, value): + return queryset.exclude(user__email=value) + + +class ProjectMembershipListNode(DjangoObjectType, MembershipListNodeMixin): + class Meta(MembershipListNodeMixin.Meta): + model = ProjectMembershipList + + memberships = DjangoFilterConnectionField( + ProjectMembershipNode, filterset_class=ProjectMembershipFilterSet, required=True + ) + + def resolve_memberships(self, info, **kwargs): + return ProjectMembershipFilterSet(kwargs).qs.filter(membership_list=self) + + +class ProjectFilterSet(FilterSet): + member = TypedFilter(field_name="membership_list__memberships__user") + + class Meta: + model = Project + fields = { + "name": ["exact", "icontains"], + } + + +class ProjectNode(DjangoObjectType): + id = graphene.ID(source="pk", required=True) + seen = graphene.Boolean(required=True) + + class Meta: + model = Project + + filterset_class = ProjectFilterSet + fields = ( + "name", + "privacy", + "description", + "updated_at", + "site_set", + "archived", + "membership_list", + ) + + interfaces = (relay.Node,) + connection_class = TerrasoConnection + + def resolve_seen(self, info): + user = info.context.user + if user.is_anonymous: + return True + return self.seen_by.filter(id=user.id).exists() + + +class ProjectPrivacy(graphene.Enum): + PRIVATE = Project.PRIVATE + PUBLIC = Project.PUBLIC + + +class ProjectAddMutation(BaseWriteMutation): + skip_field_validation = ["membership_list", "settings"] + project = graphene.Field(ProjectNode, required=True) + + model_class = Project + + class Input: + name = graphene.String(required=True) + privacy = graphene.Field(ProjectPrivacy, required=True) + description = graphene.String() + + @classmethod + def mutate_and_get_payload(cls, root, info, **kwargs): + logger = cls.get_logger() + user = info.context.user + with transaction.atomic(): + kwargs["privacy"] = kwargs["privacy"].value + result = super().mutate_and_get_payload(root, info, **kwargs) + result.project.add_manager(user) + + client_time = kwargs.get("client_time", None) + result.project.mark_seen_by(user) + if not client_time: + client_time = datetime.now() + action = log_api.CREATE + metadata = { + "name": kwargs["name"], + "privacy": kwargs["privacy"], + "description": kwargs.get("description"), + } + logger.log( + user=user, + action=action, + resource=result.project, + client_time=client_time, + metadata=metadata, + ) + return result + + +class ProjectDeleteMutation(BaseDeleteMutation): + project = graphene.Field(ProjectNode, required=True) + + model_class = Project + + class Input: + id = graphene.ID(required=True) + transfer_project_id = graphene.ID() + + @classmethod + @transaction.atomic + def mutate_and_get_payload(cls, root, info, **kwargs): + user = info.context.user + project_id = kwargs["id"] + project = cls.get_or_throw(Project, "id", project_id) + if not user.has_perm(Project.get_perm("delete"), project): + cls.not_allowed() + if "transfer_project_id" in kwargs: + transfer_project_id = kwargs["transfer_project_id"] + transfer_project = cls.get_or_throw(Project, "id", transfer_project_id) + if not user.has_perm(Project.get_perm("add"), transfer_project): + cls.not_allowed() + project_sites = project.site_set.all() + for site in project_sites: + site.project = transfer_project + Site.objects.bulk_update(project_sites, ["project"]) + result = super().mutate_and_get_payload(root, info, **kwargs) + return result + + +class ProjectArchiveMutation(BaseWriteMutation): + project = graphene.Field(ProjectNode, required=True) + + model_class = Project + + class Input: + id = graphene.ID(required=True) + archived = graphene.Boolean(required=True) + + @classmethod + @transaction.atomic + def mutate_and_get_payload(cls, root, info, **kwargs): + user = info.context.user + project_id = kwargs["id"] + project = cls.get_or_throw(Project, "id", project_id) + if not user.has_perm(Project.get_perm("archive"), project): + cls.not_allowed() + project_sites = project.site_set.all() + for site in project_sites: + site.archived = kwargs["archived"] + Site.objects.bulk_update(project_sites, ["archived"]) + result = super().mutate_and_get_payload(root, info, **kwargs) + return result + + +class ProjectUpdateMutation(BaseWriteMutation): + project = graphene.Field(ProjectNode) + + model_class = Project + + class Input: + id = graphene.ID(required=True) + name = graphene.String() + privacy = graphene.Field(ProjectPrivacy) + description = graphene.String() + + @classmethod + @transaction.atomic + def mutate_and_get_payload(cls, root, info, **kwargs): + logger = cls.get_logger() + user = info.context.user + project_id = kwargs["id"] + project = cls.get_or_throw(Project, "id", project_id) + if not user.has_perm(Project.get_perm("change"), project): + cls.not_allowed() + kwargs["privacy"] = kwargs["privacy"].value + + metadata = { + "name": kwargs["name"], + "privacy": kwargs["privacy"], + "description": kwargs["description"] if "description" in kwargs else None, + } + logger.log( + user=user, + action=log_api.CHANGE, + resource=project, + client_time=datetime.now(), + metadata=metadata, + ) + + return super().mutate_and_get_payload(root, info, **kwargs) + + +class ProjectAddUserMutation(BaseWriteMutation): + project = graphene.Field(ProjectNode, required=True) + membership = graphene.Field(MembershipNode, required=True) + + model_class = Membership + + class Input: + project_id = graphene.ID(required=True) + user_id = graphene.ID(required=True) + role = graphene.Field(UserRole, required=True) + + @classmethod + def mutate_and_get_payload(cls, root, info, project_id, user_id, role): + if role not in Project.ROLES: + raise GraphQLValidationException(message=f"Invalid role: {role}") + project = cls.get_or_throw(Project, "project_id", project_id) + user = cls.get_or_throw(User, "user_id", user_id) + request_user = info.context.user + requester_membership = project.get_membership(request_user) + if not requester_membership: + cls.not_allowed_create(model=Membership, msg="User does not belong to project") + + def validate(context): + if not rules.test_rule( + "allowed_to_add_member_to_project", + request_user, + {"project": project, "requester_membership": requester_membership}, + ): + raise ValidationError("User cannot add membership to this project") + + # add membership + try: + _, membership = project.membership_list.save_membership( + user_email=user.email, + user_role=role.value, + membership_status=Membership.APPROVED, + validation_func=validate, + ) + except ValidationError as e: + cls.not_allowed_create(model=Membership, msg=e.message) + + membership_added_signal.send(sender=cls, membership=membership, user=request_user) + + return ProjectAddUserMutation(project=project, membership=membership) + + +class ProjectDeleteUserMutation(BaseWriteMutation): + project = graphene.Field(ProjectNode, required=True) + membership = graphene.Field(MembershipNode, required=True) + + model_class = Membership + + class Input: + project_id = graphene.ID(required=True) + user_id = graphene.ID(required=True) + + @classmethod + def mutate_and_get_payload(cls, root, info, project_id, user_id): + # check if user has proper permissions + project = cls.get_or_throw(Project, "project_id", project_id) + user = cls.get_or_throw(User, "user_id", user_id) + requester = info.context.user + requester_membership = project.get_membership(requester) + if not requester_membership: + cls.not_allowed( + MutationTypes.DELETE, + msg="User cannot delete other users" " from project where they are not a member", + ) + target_membership = project.get_membership(user) + if not target_membership: + cls.not_allowed( + MutationTypes.DELETE, msg="Cannot delete a user membership that does not exist" + ) + if not rules.test_rule( + "allowed_to_delete_user_from_project", + requester, + { + "project": project, + "requester_membership": requester_membership, + "target_membership": target_membership, + }, + ): + cls.not_allowed( + MutationTypes.DELETE, msg="User not allowed to remove member from project" + ) + # remove membership + membership = project.get_membership(user) + membership.delete() + + membership_deleted_signal.send(sender=cls, membership=target_membership, user=requester) + + return ProjectDeleteUserMutation(project=project, membership=membership) + + +class ProjectUpdateUserRoleMutation(BaseWriteMutation): + model_class = Membership + + project = graphene.Field(ProjectNode, required=True) + membership = graphene.Field(MembershipNode, required=True) + + class Input: + project_id = graphene.ID(required=True) + user_id = graphene.ID(required=True) + new_role = graphene.Field(UserRole, required=True) + + @classmethod + def mutate_and_get_payload(cls, root, info, project_id, user_id, new_role): + project = cls.get_or_throw(Project, "project_id", project_id) + requester = info.context.user + + if not (requester_membership := project.get_membership(requester)): + cls.not_allowed(MutationTypes.UPDATE, msg="Requesting member not a member of project") + + target_user = cls.get_or_throw(User, "user_id", user_id) + if not (target_membership := project.get_membership(target_user)): + cls.not_allowed(MutationTypes.UPDATE, msg="Target user not a member of project") + + if not rules.test_rule( + "allowed_to_change_user_project_role", + requester, + { + "project": project, + "requester_membership": requester_membership, + "target_membership": target_membership, + "user_role": new_role, + }, + ): + cls.not_allowed( + MutationTypes.UPDATE, msg="User is not allowed to change other user role" + ) + + target_membership.user_role = new_role.value + target_membership.save() + + membership_updated_signal.send(sender=cls, membership=target_membership, user=requester) + + return ProjectUpdateUserRoleMutation(project=project, membership=target_membership) + + +class ProjectMarkSeenMutation(BaseAuthenticatedMutation): + project = graphene.Field(ProjectNode, required=True) + + class Input: + id = graphene.ID(required=True) + + @classmethod + def mutate_and_get_payload(cls, root, info, id): + user = info.context.user + project = BaseMutation.get_or_throw(Project, "id", id) + project.mark_seen_by(user) + return ProjectMarkSeenMutation(project=project) diff --git a/terraso_backend/apps/project_management/migrations/0015_remove_project_group_project_membership_list.py b/terraso_backend/apps/project_management/migrations/0015_remove_project_group_project_membership_list.py new file mode 100644 index 000000000..b3277acae --- /dev/null +++ b/terraso_backend/apps/project_management/migrations/0015_remove_project_group_project_membership_list.py @@ -0,0 +1,31 @@ +# Generated by Django 4.2.5 on 2023-09-15 15:24 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("collaboration", "0005_change_collaborator_to_editor"), + ("project_management", "0014_site_privacy_alter_project_description"), + ] + + operations = [ + migrations.AddField( + model_name="project", + name="membership_list", + field=models.OneToOneField( + null=True, + on_delete=django.db.models.deletion.CASCADE, + to="collaboration.membershiplist", + ), + preserve_default=False, + ), + migrations.AlterField( + model_name="project", + name="group", + field=models.OneToOneField( + null=True, on_delete=django.db.models.deletion.CASCADE, to="core.group" + ), + ), + ] diff --git a/terraso_backend/apps/project_management/migrations/0016_auto_20230915_1527.py b/terraso_backend/apps/project_management/migrations/0016_auto_20230915_1527.py new file mode 100644 index 000000000..8866910ee --- /dev/null +++ b/terraso_backend/apps/project_management/migrations/0016_auto_20230915_1527.py @@ -0,0 +1,57 @@ +# Generated by Django 4.2.5 on 2023-09-15 15:27 + +from django.db import migrations + +from apps.project_management import collaboration_roles + + +def migrate_forward(apps, schema_editor): + Project = apps.get_model("project_management", "Project") + MembershipList = apps.get_model("collaboration", "MembershipList") + Membership = apps.get_model("collaboration", "Membership") + # The example docs https://docs.djangoproject.com/en/4.2/ref/migration-operations/#runpython + # use the db_alias. + # probably not technically necessary because we don't use multiple DBs + db_alias = schema_editor.connection.alias + for project in Project.objects.using(db_alias).all(): + group = project.group + project.membership_list = MembershipList.objects.using(db_alias).create( + membership_type="open", + enroll_method="join", + ) + # add group memberships to membership list + for membership in group.memberships.all(): + user_role = ( + collaboration_roles.ROLE_MANAGER + if membership.user_role == "manager" + else collaboration_roles.ROLE_VIEWER + ) + + if ( + Membership.objects.using(db_alias) + .filter(user=membership.user, membership_list=project.membership_list) + .exists() + ): + # it seems like group constraints are not as strict as MembershipLists + # if there are several Memberships with the same user, just ignore + # (this was happening with my local DB instance) + continue + Membership.objects.using(db_alias).create( + user=membership.user, + membership_list=project.membership_list, + user_role=user_role, + membership_status="approved", + ) + # clean up + membership.delete() + project.group.delete() + project.group = None + project.save() + + +class Migration(migrations.Migration): + dependencies = [ + ("project_management", "0015_remove_project_group_project_membership_list"), + ] + + operations = [migrations.RunPython(migrate_forward, atomic=True)] diff --git a/terraso_backend/apps/project_management/migrations/0017_auto_20230915_1527.py b/terraso_backend/apps/project_management/migrations/0017_auto_20230915_1527.py new file mode 100644 index 000000000..d00adffd1 --- /dev/null +++ b/terraso_backend/apps/project_management/migrations/0017_auto_20230915_1527.py @@ -0,0 +1,26 @@ +# Generated by Django 4.2.5 on 2023-09-15 15:27 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("project_management", "0016_auto_20230915_1527"), + ] + + operations = [ + migrations.RemoveField( + model_name="project", + name="group", + ), + migrations.AlterField( + model_name="project", + name="membership_list", + field=models.OneToOneField( + on_delete=django.db.models.deletion.CASCADE, + to="collaboration.membershiplist", + null=False, + ), + ), + ] diff --git a/terraso_backend/apps/project_management/migrations/0018_projectmembership_projectmembershiplist_and_more.py b/terraso_backend/apps/project_management/migrations/0018_projectmembership_projectmembershiplist_and_more.py new file mode 100644 index 000000000..a2fd8deed --- /dev/null +++ b/terraso_backend/apps/project_management/migrations/0018_projectmembership_projectmembershiplist_and_more.py @@ -0,0 +1,57 @@ +# 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.5 on 2023-09-25 19:20 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("collaboration", "0005_change_collaborator_to_editor"), + ("project_management", "0017_auto_20230915_1527"), + ] + + operations = [ + migrations.CreateModel( + name="ProjectMembership", + fields=[], + options={ + "proxy": True, + "indexes": [], + "constraints": [], + }, + bases=("collaboration.membership",), + ), + migrations.CreateModel( + name="ProjectMembershipList", + fields=[], + options={ + "proxy": True, + "indexes": [], + "constraints": [], + }, + bases=("collaboration.membershiplist",), + ), + migrations.AlterField( + model_name="project", + name="membership_list", + field=models.OneToOneField( + on_delete=django.db.models.deletion.CASCADE, + to="project_management.projectmembershiplist", + ), + ), + ] diff --git a/terraso_backend/apps/project_management/migrations/0015_project_seen_by_site_seen_by.py b/terraso_backend/apps/project_management/migrations/0019_project_seen_by_site_seen_by.py similarity index 88% rename from terraso_backend/apps/project_management/migrations/0015_project_seen_by_site_seen_by.py rename to terraso_backend/apps/project_management/migrations/0019_project_seen_by_site_seen_by.py index e1c3a1bb2..bbd9d837b 100644 --- a/terraso_backend/apps/project_management/migrations/0015_project_seen_by_site_seen_by.py +++ b/terraso_backend/apps/project_management/migrations/0019_project_seen_by_site_seen_by.py @@ -7,7 +7,7 @@ class Migration(migrations.Migration): dependencies = [ migrations.swappable_dependency(settings.AUTH_USER_MODEL), - ("project_management", "0014_site_privacy_alter_project_description"), + ("project_management", "0018_projectmembership_projectmembershiplist_and_more"), ] operations = [ diff --git a/terraso_backend/apps/project_management/models/__init__.py b/terraso_backend/apps/project_management/models/__init__.py index 0d550f30c..2f9eb3f7e 100644 --- a/terraso_backend/apps/project_management/models/__init__.py +++ b/terraso_backend/apps/project_management/models/__init__.py @@ -1,4 +1,4 @@ -from .projects import Project, ProjectSettings +from .projects import Project, ProjectMembership, ProjectMembershipList, ProjectSettings from .sites import Site -__all__ = ["Project", "ProjectSettings", "Site"] +__all__ = ["Project", "ProjectSettings", "Site", "ProjectMembership", "ProjectMembershipList"] diff --git a/terraso_backend/apps/project_management/models/projects.py b/terraso_backend/apps/project_management/models/projects.py index 2cc0f1916..ff36f8902 100644 --- a/terraso_backend/apps/project_management/models/projects.py +++ b/terraso_backend/apps/project_management/models/projects.py @@ -12,14 +12,18 @@ # # 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 secrets - from django.db import models from django.utils.translation import gettext_lazy as _ -from apps.core.models import Group, User +from apps.collaboration.models import Membership, MembershipList +from apps.core.models import User from apps.core.models.commons import BaseModel from apps.project_management import permission_rules +from apps.project_management.collaboration_roles import ( + ROLE_CONTRIBUTOR, + ROLE_MANAGER, + ROLE_VIEWER, +) class ProjectSettings(BaseModel): @@ -30,6 +34,20 @@ class Meta(BaseModel.Meta): member_can_add_site_to_project = models.BooleanField(default=False) +class ProjectMembership(Membership): + """A proxy class created soley for graphene schema reasons""" + + class Meta: + proxy = True + + +class ProjectMembershipList(MembershipList): + """A proxy class created soley for graphql schema reasons""" + + class Meta: + proxy = True + + class Project(BaseModel): class Meta(BaseModel.Meta): abstract = False @@ -42,6 +60,8 @@ class Meta(BaseModel.Meta): "archive": permission_rules.allowed_to_archive_project, } + ROLES = (ROLE_VIEWER, ROLE_CONTRIBUTOR, ROLE_MANAGER) + PRIVATE = "private" PUBLIC = "public" DEFAULT_PRIVACY_STATUS = PRIVATE @@ -50,7 +70,7 @@ class Meta(BaseModel.Meta): name = models.CharField(max_length=200) description = models.CharField(max_length=512, default="", blank=True) - group = models.OneToOneField(Group, on_delete=models.CASCADE) + membership_list = models.OneToOneField(ProjectMembershipList, on_delete=models.CASCADE) privacy = models.CharField( max_length=32, choices=PRIVACY_STATUS, default=DEFAULT_PRIVACY_STATUS ) @@ -69,8 +89,8 @@ def default_settings(): def save(self, *args, **kwargs): if not hasattr(self, "settings"): self.settings = self.default_settings() - if not hasattr(self, "group"): - self.group = self.create_default_group(self.name) + if not hasattr(self, "membership_list"): + self.membership_list = self.create_membership_list() return super(Project, self).save(*args, **kwargs) archived = models.BooleanField( @@ -78,34 +98,55 @@ def save(self, *args, **kwargs): ) @staticmethod - def create_default_group(name: str): + def create_membership_list() -> MembershipList: """Creates a default group for a project""" - group_name = f"project_group_{name}_{secrets.token_hex(6)}" - return Group.objects.create( - name=group_name, - membership_type=Group.MEMBERSHIP_TYPE_OPEN, - enroll_method=Group.ENROLL_METHOD_INVITE, + return MembershipList.objects.create( + membership_type=MembershipList.MEMBERSHIP_TYPE_OPEN, + enroll_method=MembershipList.ENROLL_METHOD_JOIN, ) def is_manager(self, user: User) -> bool: - return self.managers.filter(id=user.id).exists() + return self.manager_memberships.filter(user=user).exists() + + def is_viewer(self, user: User) -> bool: + return self.viewer_memberships.filter(user=user).exists() + + def is_contributor(self, user: User) -> bool: + return self.contributor_memberships.filter(user=user).exists() def is_member(self, user: User) -> bool: - return self.members.filter(id=user.id).exists() + return self.membership_list.is_member(user) @property - def managers(self): - return self.group.group_managers + def manager_memberships(self): + return self.membership_list.memberships.by_role(ROLE_MANAGER) @property - def members(self): - return self.group.group_members + def viewer_memberships(self): + return self.membership_list.memberships.by_role(ROLE_VIEWER) + + @property + def contributor_memberships(self): + return self.membership_list.memberships.by_role(ROLE_CONTRIBUTOR) def add_manager(self, user: User): - return self.group.add_manager(user) + return self.add_user_with_role(user, ROLE_MANAGER) + + def add_viewer(self, user: User): + return self.add_user_with_role(user, ROLE_VIEWER) + + def add_user_with_role(self, user: User, role: str): + assert role in self.ROLES + return Membership.objects.create( + membership_list=self.membership_list, + user=user, + membership_status=Membership.APPROVED, + user_role=role, + pending_email=None, + ) - def add_member(self, user: User): - return self.group.add_member(user) + def get_membership(self, user: User): + return self.membership_list.memberships.filter(user=user).first() def mark_seen_by(self, user: User): self.seen_by.add(user) diff --git a/terraso_backend/apps/project_management/models/sites.py b/terraso_backend/apps/project_management/models/sites.py index 367d8d45b..afc0515b0 100644 --- a/terraso_backend/apps/project_management/models/sites.py +++ b/terraso_backend/apps/project_management/models/sites.py @@ -107,7 +107,7 @@ def filter_only_sites_user_owner_or_member(user: User, queryset): return queryset.filter( Q(owner=user) | Q( - project__group__memberships__user=user, - project__group__memberships__membership_status=Membership.APPROVED, + project__membership_list__memberships__user=user, + project__membership_list__memberships__membership_status=Membership.APPROVED, ) ) diff --git a/terraso_backend/apps/project_management/permission_rules.py b/terraso_backend/apps/project_management/permission_rules.py index d0eaf8845..cd7cbbf46 100644 --- a/terraso_backend/apps/project_management/permission_rules.py +++ b/terraso_backend/apps/project_management/permission_rules.py @@ -14,6 +14,8 @@ # along with this program. If not, see https://www.gnu.org/licenses/. import rules +from .collaboration_roles import ROLE_MANAGER + @rules.predicate def allowed_to_change_project(user, project): @@ -56,3 +58,45 @@ def allowed_to_add_to_project(user, project): @rules.predicate def allowed_to_archive_project(user, project): return project.is_manager(user) + + +@rules.predicate +def allowed_to_add_member_to_project(user, context): + project = context["project"] + requester_membership = context["requester_membership"] + return ( + requester_membership.membership_list == project.membership_list + and requester_membership.user_role == ROLE_MANAGER + ) + + +rules.add_rule("allowed_to_add_member_to_project", allowed_to_add_member_to_project) + + +@rules.predicate +def allowed_to_delete_user_from_project(user, context): + project = context["project"] + requester_membership = context["requester_membership"] + target_membership = context["target_membership"] + return project.membership_list == requester_membership.membership_list and ( + user == target_membership.user or requester_membership.user_role == ROLE_MANAGER + ) + + +rules.add_rule("allowed_to_delete_user_from_project", allowed_to_delete_user_from_project) + + +@rules.predicate +def allowed_to_change_user_project_role(user, context): + project = context["project"] + requester_membership = context["requester_membership"] + target_membership = context["target_membership"] + return ( + project.membership_list + == requester_membership.membership_list + == target_membership.membership_list + and requester_membership.user_role == ROLE_MANAGER + ) + + +rules.add_rule("allowed_to_change_user_project_role", allowed_to_change_user_project_role) diff --git a/terraso_backend/apps/project_management/signals.py b/terraso_backend/apps/project_management/signals.py index 472a2827e..295c9f6f0 100644 --- a/terraso_backend/apps/project_management/signals.py +++ b/terraso_backend/apps/project_management/signals.py @@ -19,14 +19,18 @@ from apps.audit_logs import api as audit_log_api from apps.audit_logs import services -from apps.graphql.signals import membership_added_signal, membership_updated_signal +from apps.graphql.signals import ( + membership_added_signal, + membership_deleted_signal, + membership_updated_signal, +) audit_logger = services.new_audit_logger() def _handle_membership_log(user, action, membership, client_time): try: - project = membership.group.project + project = membership.membership_list.project except Exception: # No project for membership, do nothing return @@ -60,3 +64,12 @@ def handle_membership_updated(sender, **kwargs): client_time = datetime.now() _handle_membership_log(user, audit_log_api.CHANGE, membership, client_time) + + +@receiver(membership_deleted_signal) +def handle_membership_deleted(sender, **kwargs): + membership = kwargs["membership"] + user = kwargs["user"] + client_time = datetime.now() + + _handle_membership_log(user, audit_log_api.DELETE, membership, client_time) diff --git a/terraso_backend/apps/story_map/collaboration_roles.py b/terraso_backend/apps/story_map/collaboration_roles.py index 6539458a0..5dbafa3fe 100644 --- a/terraso_backend/apps/story_map/collaboration_roles.py +++ b/terraso_backend/apps/story_map/collaboration_roles.py @@ -13,4 +13,5 @@ # 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_EDITOR = "editor" diff --git a/terraso_backend/tests/conftest.py b/terraso_backend/tests/conftest.py index 2b2da2715..b73a6dca5 100644 --- a/terraso_backend/tests/conftest.py +++ b/terraso_backend/tests/conftest.py @@ -22,9 +22,10 @@ from pyproj import CRS, Transformer from apps.auth.services import JWTService +from apps.collaboration.models import Membership from apps.core.gis.utils import DEFAULT_CRS from apps.core.models import User -from apps.project_management.models import Project, ProjectSettings, Site +from apps.project_management.models import Project, Site pytestmark = pytest.mark.django_db @@ -91,17 +92,15 @@ def site_creator(site: Site) -> User: @pytest.fixture def project() -> Project: - group = Project.create_default_group("test_group") - project = mixer.blend(Project, group=group) + project = mixer.blend(Project) user = mixer.blend(User) project.add_manager(user) - ProjectSettings.objects.create(project=project) return project @pytest.fixture def project_manager(project: Project) -> User: - return project.managers.first() + return project.manager_memberships.first().user @pytest.fixture @@ -113,5 +112,10 @@ def project_with_sites(project: Project) -> Project: @pytest.fixture def project_user(project: Project) -> User: user = mixer.blend(User) - project.add_member(user) + Membership.objects.create( + user=user, + membership_list=project.membership_list, + user_role="viewer", + membership_status=Membership.APPROVED, + ) return user diff --git a/terraso_backend/tests/graphql/mutations/test_membership_mutations.py b/terraso_backend/tests/graphql/mutations/test_membership_mutations.py index c07da9f07..72865ba53 100644 --- a/terraso_backend/tests/graphql/mutations/test_membership_mutations.py +++ b/terraso_backend/tests/graphql/mutations/test_membership_mutations.py @@ -17,10 +17,9 @@ from unittest import mock import pytest -from graphene_django.utils.testing import graphql_query from mixer.backend.django import mixer -from apps.core.models import Membership, User +from apps.core.models import Membership pytestmark = pytest.mark.django_db @@ -648,41 +647,3 @@ def test_membership_delete_by_last_manager(client_query, memberships, users): assert "errors" in response["data"]["deleteMembership"] assert "delete_not_allowed" in response["data"]["deleteMembership"]["errors"][0]["message"] - - -ADD_MEMBERSHIP_MUTATION = """ -mutation addMembership($input: MembershipAddMutationInput!){ - addMembership(input: $input) { - membership { - id - userRole - user { - email - } - group { - slug - } - } - errors - } -} -""" - - -def test_user_not_in_project_cannot_join_project_group(client, project): - outside_user = mixer.blend(User) - client.force_login(outside_user) - response = graphql_query( - ADD_MEMBERSHIP_MUTATION, - variables={ - "input": { - "userEmail": outside_user.email, - "groupSlug": project.group.slug, - "userRole": Membership.ROLE_MEMBER, - } - }, - client=client, - ).json() - - assert "errors" in response["data"]["addMembership"] - assert "create_not_allowed" in response["data"]["addMembership"]["errors"][0]["message"] diff --git a/terraso_backend/tests/graphql/mutations/test_projects.py b/terraso_backend/tests/graphql/mutations/test_projects.py index 4a893dc13..bb5bb4281 100644 --- a/terraso_backend/tests/graphql/mutations/test_projects.py +++ b/terraso_backend/tests/graphql/mutations/test_projects.py @@ -37,7 +37,7 @@ def test_create_project(client, user): assert "errors" not in content id = content["data"]["addProject"]["project"]["id"] project = Project.objects.get(pk=id) - assert list(project.managers.all()) == [user] + assert list([mb.user for mb in project.manager_memberships.all()]) == [user] assert project.description == "A test project" logs = Log.objects.all() @@ -53,115 +53,6 @@ def test_create_project(client, user): assert log_result.metadata == expected_metadata -ADD_MEMBERSHIP_GRAPHQL = """ - mutation addUserToProject($input: MembershipAddMutationInput!) { - addMembership(input: $input) { - membership { - id - } - } - } -""" - - -def test_add_user_to_project(client, project, project_manager, user): - client.force_login(project_manager) - response = graphql_query( - ADD_MEMBERSHIP_GRAPHQL, - variables={ - "input": { - "userEmail": user.email, - "groupSlug": project.group.slug, - "userRole": "member", - } - }, - client=client, - ) - content = json.loads(response.content) - assert "errors" not in content and "errors" not in content["data"]["addMembership"] - assert project.is_member(user) - - -def test_add_user_to_project_audit_log(client, project, project_manager, user): - client.force_login(project_manager) - - assert project_manager.id != user.id - - response = graphql_query( - ADD_MEMBERSHIP_GRAPHQL, - variables={ - "input": { - "userEmail": user.email, - "groupSlug": project.group.slug, - "userRole": "member", - } - }, - client=client, - ) - - assert response.status_code == 200 - - membership = project.group.memberships.filter(user=user).first() - - logs = Log.objects.all() - assert len(logs) == 1 - log_result = logs[0] - assert log_result.event == CREATE.value - assert log_result.user_human_readable == project_manager.full_name() - assert log_result.resource_object == membership - expected_metadata = { - "user_email": user.email, - "user_role": "member", - "project_id": str(project.id), - } - assert log_result.metadata == expected_metadata - - -def test_update_user_to_project_audit_log(client, project, project_manager, user): - client.force_login(project_manager) - - assert project_manager.id != user.id - - project.group.add_member(user) - membership = project.group.memberships.filter(user=user).first() - - response = graphql_query( - """ - mutation updateMembership($input: MembershipUpdateMutationInput!) { - updateMembership(input: $input) { - membership { - id - } - } - } - """, - variables={ - "input": { - "id": str(membership.id), - "userRole": "manager", - } - }, - client=client, - ) - - assert response.status_code == 200 - - membership = project.group.memberships.filter(user=user).first() - - logs = Log.objects.all() - assert len(logs) == 1 - log_result = logs[0] - assert log_result.event == CHANGE.value - assert log_result.user_human_readable == project_manager.full_name() - assert log_result.resource_object == membership - expected_metadata = { - "user_email": user.email, - "user_role": "manager", - "project_id": str(project.id), - } - assert log_result.metadata == expected_metadata - - DELETE_PROJECT_GRAPHQL = """ mutation($input: ProjectDeleteMutationInput!) { deleteProject(input: $input) { @@ -185,11 +76,9 @@ def test_delete_project(project_with_sites, client, project_manager): assert not Site.objects.filter(id__in=site_ids).exists() -def test_delete_project_user_not_manager(project, client): - user = mixer.blend(User) - project.add_member(user) +def test_delete_project_user_not_manager(project, client, project_user): input = {"id": str(project.id)} - client.force_login(user) + client.force_login(project_user) response = graphql_query(DELETE_PROJECT_GRAPHQL, input_data=input, client=client) content = json.loads(response.content) assert "errors" in content or "errors" in content["data"]["deleteProject"] @@ -202,7 +91,7 @@ def test_delete_project_transfer_sites(is_manager, project_with_sites, client, p if is_manager: other_project.add_manager(project_manager) else: - other_project.add_member(project_manager) + other_project.add_viewer(project_manager) input = {"id": str(project_with_sites.id), "transferProjectId": str(other_project.id)} site_ids = [site.id for site in project_with_sites.site_set.all()] client.force_login(project_manager) @@ -239,11 +128,9 @@ def test_archive_project(archived, project_with_sites, client, project_manager): assert Site.objects.filter(id__in=site_ids, archived=archived).exists() -def test_archive_project_user_not_manager(project, client): - user = mixer.blend(User) - project.add_member(user) +def test_archive_project_user_not_manager(project, client, project_user): input = {"id": str(project.id), "archived": True} - client.force_login(user) + client.force_login(project_user) response = graphql_query(ARCHIVE_PROJECT_GRAPHQL, input_data=input, client=client) content = json.loads(response.content) assert "errors" in content or "errors" in content["data"]["archiveProject"] @@ -298,17 +185,223 @@ def test_update_project_audit_log(project, client, project_manager): assert log_result.metadata == expected_metadata -def test_update_project_user_not_manager(project, client): - user = mixer.blend(User) - project.add_member(user) +def test_update_project_user_not_manager(project, client, project_user): input = {"id": str(project.id), "name": "test_name", "privacy": "PRIVATE"} - client.force_login(user) + client.force_login(project_user) response = graphql_query(UPDATE_PROJECT_GRAPHQL, input_data=input, client=client) error_result = response.json()["data"]["updateProject"]["errors"][0]["message"] json_error = json.loads(error_result) assert json_error[0]["code"] == "change_not_allowed" +ADD_USER_GRAPHQL = """ +mutation addUser($input: ProjectAddUserMutationInput!) { + addUserToProject(input: $input) { + project { + id + } + membership { + user { + id + } + } + } +} +""" + + +def test_add_user_to_project(project, project_manager, client): + user = mixer.blend(User) + client.force_login(project_manager) + input_data = {"projectId": str(project.id), "userId": str(user.id), "role": "viewer"} + response = graphql_query(ADD_USER_GRAPHQL, input_data=input_data, client=client) + payload = response.json() + assert "errors" not in payload and "errors" not in payload["data"] + data = payload["data"]["addUserToProject"] + assert data["membership"]["user"]["id"] == str(user.id) + project.refresh_from_db() + assert project.viewer_memberships.filter(user=user).exists() + + +def test_add_user_to_project_audit_log(client, project, project_manager, user): + client.force_login(project_manager) + + assert project_manager.id != user.id + + response = graphql_query( + ADD_USER_GRAPHQL, + variables={ + "input": { + "projectId": str(project.id), + "userId": str(user.id), + "role": "viewer", + } + }, + client=client, + ) + + assert response.status_code == 200 + + membership = project.viewer_memberships.first() + + logs = Log.objects.all() + assert len(logs) == 1 + log_result = logs[0] + assert log_result.event == CREATE.value + assert log_result.user_human_readable == project_manager.full_name() + assert log_result.resource_object == membership + expected_metadata = { + "user_email": user.email, + "user_role": "viewer", + "project_id": str(project.id), + } + assert log_result.metadata == expected_metadata + + +def test_add_user_to_project_bad_roles(project, project_manager, client): + user = mixer.blend(User) + client.force_login(project_manager) + input_data = {"projectId": str(project.id), "userId": str(user.id), "role": "garbage"} + response = graphql_query(ADD_USER_GRAPHQL, input_data=input_data, client=client) + payload = response.json() + assert "errors" in payload + + +DELETE_USER_GRAPHQL = """ +mutation deleteUser($input: ProjectDeleteUserMutationInput!) { + deleteUserFromProject(input: $input) { + project { + id + } + membership { + user { + id + } + } + } +} +""" + + +def test_delete_user_from_project_manager(project, project_manager, project_user, client): + manager_membership = project.get_membership(project_manager) + client.force_login(project_manager) + input_data = {"projectId": str(project.id), "userId": str(project_user.id)} + response = graphql_query(DELETE_USER_GRAPHQL, input_data=input_data, client=client) + payload = response.json() + assert "errors" not in payload + assert payload["data"]["deleteUserFromProject"]["membership"]["user"]["id"] == str( + project_user.id + ) + project.refresh_from_db() + assert list(project.membership_list.memberships.all()) == [manager_membership] + + +def test_delete_user_from_project_delete_self(project, project_user, client): + client.force_login(project_user) + input_data = {"projectId": str(project.id), "userId": str(project_user.id)} + response = graphql_query(DELETE_USER_GRAPHQL, input_data=input_data, client=client) + payload = response.json() + assert "errors" not in payload + assert payload["data"]["deleteUserFromProject"]["membership"]["user"]["id"] == str( + project_user.id + ) + project.refresh_from_db() + assert project_user in project.membership_list.members.all() + + +def test_delete_user_from_project_not_manager(project, project_user, client): + other_user = mixer.blend(User) + project.add_user_with_role(other_user, "contributor") + client.force_login(project_user) + input_data = {"projectId": str(project.id), "userId": str(other_user.id)} + response = graphql_query(DELETE_USER_GRAPHQL, input_data=input_data, client=client) + payload = response.json() + assert "errors" in payload + + +def test_delete_user_from_project_not_member(project, project_user, client): + other_user = mixer.blend(User) + client.force_login(other_user) + input_data = {"projectId": str(project.id), "userId": str(project_user.id)} + response = graphql_query(DELETE_USER_GRAPHQL, input_data=input_data, client=client) + payload = response.json() + assert "errors" in payload + + +UPDATE_PROJECT_ROLE_GRAPHQL = """ +mutation updateProjectRole($input: ProjectUpdateUserRoleMutationInput!) { + updateUserRoleInProject(input: $input) { + project { + id + } + membership { + userRole + user { + id + } + } + } +} +""" + + +def test_update_project_role_manager(project, project_manager, project_user, client): + client.force_login(project_manager) + input_data = { + "projectId": str(project.id), + "userId": str(project_user.id), + "newRole": "contributor", + } + response = graphql_query(UPDATE_PROJECT_ROLE_GRAPHQL, input_data=input_data, client=client) + payload = response.json() + assert "errors" not in payload + assert payload["data"]["updateUserRoleInProject"]["membership"]["userRole"] == "contributor" + assert project.is_contributor(project_user) + assert not project.is_viewer(project_user) + assert response.status_code == 200 + + membership = project.get_membership(user=project_user) + + logs = Log.objects.all() + assert len(logs) == 1 + log_result = logs[0] + assert log_result.event == CHANGE.value + assert log_result.user_human_readable == project_manager.full_name() + assert log_result.resource_object == membership + expected_metadata = { + "user_email": project_user.email, + "user_role": "contributor", + "project_id": str(project.id), + } + assert log_result.metadata == expected_metadata + + +def test_update_project_role_not_manager(project, project_user, client): + client.force_login(project_user) + input_data = { + "projectId": str(project.id), + "userId": str(project_user.id), + "newRole": "contributor", + } + response = graphql_query(UPDATE_PROJECT_ROLE_GRAPHQL, input_data=input_data, client=client) + payload = response.json() + assert "errors" in payload + + +def test_update_project_role_user_not_in_project(project, project_user, client): + other_user = mixer.blend(User) + client.force_login(other_user) + input_data = { + "projectId": str(project.id), + "userId": str(project_user.id), + "newRole": "contributor", + } + response = graphql_query(UPDATE_PROJECT_ROLE_GRAPHQL, input_data=input_data, client=client) + payload = response.json() + assert "errors" in payload + + def test_mark_project_seen(client, user): client.force_login(user) response = graphql_query( diff --git a/terraso_backend/tests/graphql/mutations/test_sites.py b/terraso_backend/tests/graphql/mutations/test_sites.py index ef442c5c7..1cdc584cf 100644 --- a/terraso_backend/tests/graphql/mutations/test_sites.py +++ b/terraso_backend/tests/graphql/mutations/test_sites.py @@ -136,9 +136,8 @@ def test_update_site_in_project(client, project, project_manager, site): assert log_result.metadata["project_id"] == str(project.id) -def test_adding_site_to_project_user_not_manager(client, project, site, user): - site_creator = mixer.blend(User) - project.add_member(user) +def test_adding_site_to_project_user_not_manager(client, project, site, project_user): + site_creator = project_user client.force_login(site_creator) response = graphql_query( UPDATE_SITE_QUERY, diff --git a/terraso_backend/tests/graphql/test_projects_query.py b/terraso_backend/tests/graphql/test_projects_query.py index 19677d7d3..4913aea62 100644 --- a/terraso_backend/tests/graphql/test_projects_query.py +++ b/terraso_backend/tests/graphql/test_projects_query.py @@ -22,8 +22,6 @@ def test_query_by_member(client, project, project_user): project2 = Project(name="2") - project2.group = project2.create_default_group("2") - project2.settings = project2.default_settings() project2.save() query = """ { @@ -32,7 +30,7 @@ def test_query_by_member(client, project, project_user): node { id name - group { + membershipList { id } } diff --git a/terraso_backend/tests/graphql/test_sites_query.py b/terraso_backend/tests/graphql/test_sites_query.py index 4cc169db9..ec9dbf816 100644 --- a/terraso_backend/tests/graphql/test_sites_query.py +++ b/terraso_backend/tests/graphql/test_sites_query.py @@ -131,8 +131,6 @@ def test_query_by_project(client, project, project_manager, site): def test_query_by_project_member(client, project, site, project_user): project2 = Project(name="2") - project2.group = project2.create_default_group("2") - project2.settings = project2.default_settings() project2.save() site.project = project site.owner = None @@ -190,7 +188,6 @@ def test_query_by_owner(client, project, site, user): def test_query_site_permissions(client, client_query, project, project_manager, site, user): - project.add_manager(project_manager) site.project = None site.owner = project_manager site.save()