Skip to content

Implement delete team member endpoint(TS-2315) #1118

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion django/thunderstore/api/cyberstorm/services/team.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from thunderstore.core.exceptions import PermissionValidationError
from thunderstore.core.types import UserType
from thunderstore.repository.models import Namespace, Team
from thunderstore.repository.models import Namespace, Team, TeamMember
from thunderstore.repository.models.team import TeamMemberRole


Expand All @@ -31,3 +31,14 @@ def create_team(user: UserType, team_name: str) -> Team:
team = Team.objects.create(name=team_name)
team.add_member(user=user, role=TeamMemberRole.owner)
return team


@transaction.atomic
def remove_team_member(agent: UserType, team_member: TeamMember) -> None:
team_member.team.ensure_user_can_access(agent)

if team_member.user != agent:
team_member.team.ensure_user_can_manage_members(agent)
team_member.team.ensure_member_can_be_removed(team_member)

team_member.delete()
Original file line number Diff line number Diff line change
@@ -1,35 +1,37 @@
import pytest
from django.core.exceptions import ValidationError
from django.http import Http404

from thunderstore.api.cyberstorm.services import team as team_services
from thunderstore.repository.models import Namespace, Team, TeamMemberRole
from conftest import TestUserTypes
from thunderstore.api.cyberstorm.services.team import (
create_team,
disband_team,
remove_team_member,
)
from thunderstore.core.exceptions import PermissionValidationError
from thunderstore.core.factories import UserFactory
from thunderstore.repository.models import Namespace, Team, TeamMember, TeamMemberRole


@pytest.mark.django_db
def test_disband_team_success(team_owner):
team_pk = team_owner.team.pk
team_services.disband_team(team_owner.user, team_owner.team.name)
disband_team(team_owner.user, team_owner.team.name)
assert not Team.objects.filter(pk=team_pk).exists()


@pytest.mark.django_db
def test_disband_team_team_not_found(user):
with pytest.raises(Http404):
team_services.disband_team(user, "NonExistentTeam")


@pytest.mark.django_db
def test_disband_team_user_cannot_access_team(user):
team = Team.objects.create(name="TestTeam")
with pytest.raises(ValidationError, match="Must be a member to access team"):
team_services.disband_team(user, team.name)
error_msg = "Must be a member to access team"
with pytest.raises(PermissionValidationError, match=error_msg):
disband_team(user, team.name)


@pytest.mark.django_db
def test_disband_team_user_cannot_disband(team_member):
with pytest.raises(ValidationError, match="Must be an owner to disband team"):
team_services.disband_team(team_member.user, team_member.team.name)
error_msg = "Must be an owner to disband team"
with pytest.raises(PermissionValidationError, match=error_msg):
disband_team(team_member.user, team_member.team.name)


@pytest.mark.django_db
Expand All @@ -38,7 +40,7 @@ def test_create_team_name_exists_in_team(user):

error_msg = "A team with the provided name already exists"
with pytest.raises(ValidationError, match=error_msg):
team_services.create_team(user, "existing_team")
create_team(user, "existing_team")


@pytest.mark.django_db
Expand All @@ -47,23 +49,88 @@ def test_create_team_name_exists_in_namespace(user):

error_msg = "A namespace with the provided name already exists"
with pytest.raises(ValidationError, match=error_msg):
team_services.create_team(user, "existing_namespace")
create_team(user, "existing_namespace")


@pytest.mark.django_db
def test_create_team_user_is_service_account(service_account):
service_account_user = service_account.user

error_msg = "Service accounts cannot create teams"
with pytest.raises(ValidationError, match=error_msg):
team_services.create_team(service_account_user, "new_team")
with pytest.raises(PermissionValidationError, match=error_msg):
create_team(service_account_user, "new_team")


@pytest.mark.django_db
def test_create_team_success(user):
team_name = "new_team"
team = team_services.create_team(user, team_name)
team = create_team(user, team_name)

assert Team.objects.filter(name=team_name).exists()
assert team.name == team_name
assert team.members.filter(user=user, role=TeamMemberRole.owner).exists()


@pytest.mark.django_db
@pytest.mark.parametrize("user_type", TestUserTypes.options())
def test_remove_team_member_user_types(user_type: str, team_member):
user = TestUserTypes.get_user_by_type(user_type)
team_member_pk = team_member.pk

user_type_result = {
TestUserTypes.no_user: (False, PermissionValidationError),
TestUserTypes.unauthenticated: (False, PermissionValidationError),
TestUserTypes.regular_user: (True, None),
TestUserTypes.deactivated_user: (False, PermissionValidationError),
TestUserTypes.service_account: (False, PermissionValidationError),
TestUserTypes.site_admin: (True, None),
TestUserTypes.superuser: (True, None),
}

should_raise_error = user_type_result[user_type][0] is False

if not user_type in [TestUserTypes.no_user, TestUserTypes.unauthenticated]:
team_member.team.add_member(user=user, role=TeamMemberRole.owner)

if should_raise_error:

with pytest.raises(user_type_result[user_type][1]):
remove_team_member(agent=user, team_member=team_member)
else:
remove_team_member(agent=user, team_member=team_member)
assert not TeamMember.objects.filter(pk=team_member_pk).exists()


@pytest.mark.django_db
def test_remove_team_member_success(team_owner, user):
team_owner.team.add_member(user=user, role=TeamMemberRole.member)
team_member = TeamMember.objects.get(user=user)

remove_team_member(agent=team_owner.user, team_member=team_member)
assert not TeamMember.objects.filter(user=user).exists()


@pytest.mark.django_db
def test_remove_team_member_user_cannot_access(user, team_member):
error_msg = "Must be a member to access team"
with pytest.raises(PermissionValidationError, match=error_msg):
remove_team_member(agent=user, team_member=team_member)


@pytest.mark.django_db
def test_remove_team_member_user_cannot_manage_members(user, team):
user2 = UserFactory()

team.add_member(user=user, role=TeamMemberRole.member)
team.add_member(user=user2, role=TeamMemberRole.member)
team_member = TeamMember.objects.get(user=user2)

error_msg = "Must be an owner to manage team members"
with pytest.raises(PermissionValidationError, match=error_msg):
remove_team_member(agent=user, team_member=team_member)


@pytest.mark.django_db
def test_remove_team_member_cannot_remove_last_owner(team_owner):
with pytest.raises(ValidationError, match="Cannot remove last owner from team"):
remove_team_member(agent=team_owner.user, team_member=team_owner)
134 changes: 134 additions & 0 deletions django/thunderstore/api/cyberstorm/tests/test_remove_team_member.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
import pytest
from rest_framework.test import APIClient

from conftest import TestUserTypes
from thunderstore.core.types import UserType
from thunderstore.repository.factories import TeamFactory, TeamMemberFactory
from thunderstore.repository.models.team import Team, TeamMember, TeamMemberRole


def get_remove_team_member_url(team_name: str, team_member: str) -> str:
return f"/api/cyberstorm/team/{team_name}/member/{team_member}/remove/"


def make_request(api_client: APIClient, team_name: str, team_member: str):
return api_client.delete(
get_remove_team_member_url(team_name, team_member),
content_type="application/json",
)


@pytest.mark.django_db
@pytest.mark.parametrize("user_type", TestUserTypes.options())
def test_remove_team_member_user_roles(user_type, api_client, team_member):
user = TestUserTypes.get_user_by_type(user_type)
team_member_pk = team_member.pk

user_type_result = {
TestUserTypes.no_user: 401,
TestUserTypes.unauthenticated: 401,
TestUserTypes.regular_user: 204,
TestUserTypes.deactivated_user: 403,
TestUserTypes.service_account: 403,
TestUserTypes.site_admin: 204,
TestUserTypes.superuser: 204,
}

if not user_type in [TestUserTypes.no_user, TestUserTypes.unauthenticated]:
team_member.team.add_member(user=user, role=TeamMemberRole.owner)
api_client.force_authenticate(user)

response = make_request(
api_client, team_member.team.name, team_member.user.username
)

assert response.status_code == user_type_result[user_type]

if response.status_code == 204:
assert not TeamMember.objects.filter(pk=team_member_pk).exists()
else:
assert TeamMember.objects.filter(pk=team_member_pk).exists()


@pytest.mark.django_db
def test_remove_team_member_success(api_client: APIClient, user: UserType, team: Team):
TeamMemberFactory(team=team, user=user, role="owner")
api_client.force_authenticate(user)

just_a_member = TeamMemberFactory(team=team, role="member")
response = make_request(api_client, team.name, just_a_member.user.username)

assert response.status_code == 204


@pytest.mark.django_db
def test_remove_member_fail_because_user_is_not_a_member_in_team(
api_client: APIClient,
user: UserType,
team: Team,
):
TeamMemberFactory(team=team, user=user, role="owner")
api_client.force_authenticate(user)

another_team = TeamFactory()
member_in_another_team = TeamMemberFactory(team=another_team, role="owner")

response = make_request(api_client, team.name, member_in_another_team.user.username)

assert response.status_code == 404
assert response.json() == {"detail": "Not found."}


@pytest.mark.django_db
def test_remove_fail_team_does_not_exist(
api_client: APIClient,
user: UserType,
):
api_client.force_authenticate(user)

response = make_request(api_client, "nonexistent", user.username)

assert response.status_code == 404
assert response.json() == {"detail": "Not found."}


@pytest.mark.django_db
def test_remove_fail_user_is_not_authenticated(
api_client: APIClient,
user: UserType,
team: Team,
):
TeamMemberFactory(team=team, user=user, role="owner")
just_a_member = TeamMemberFactory(team=team, role="member")

response = make_request(api_client, team.name, just_a_member.user.username)
expected_response = {"detail": "Authentication credentials were not provided."}

assert response.status_code == 401
assert response.json() == expected_response


@pytest.mark.django_db
def test_remove_fail_no_permission_to_access_team(
api_client: APIClient, user: UserType, team: Team
):
api_client.force_authenticate(user)
owner = TeamMemberFactory(team=team, role="owner")

response = make_request(api_client, team.name, owner.user.username)
expected_response = {"non_field_errors": ["Must be a member to access team"]}

assert response.status_code == 403
assert response.json() == expected_response


@pytest.mark.django_db
def test_remove_fail_last_owner(api_client: APIClient, user: UserType, team: Team):
TeamMemberFactory(team=team, user=user, role="owner")
api_client.force_authenticate(user)

response = make_request(api_client, team.name, user.username)
expected_response = {"non_field_errors": ["Cannot remove last owner from team"]}

assert response.status_code == 400
assert response.json() == expected_response
2 changes: 2 additions & 0 deletions django/thunderstore/api/cyberstorm/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from .package_version_list import PackageVersionListAPIView
from .team import (
DisbandTeamAPIView,
RemoveTeamMemberAPIView,
TeamAPIView,
TeamCreateAPIView,
TeamMemberAddAPIView,
Expand Down Expand Up @@ -49,4 +50,5 @@
"UpdatePackageListingCategoriesAPIView",
"RejectPackageListingAPIView",
"ApprovePackageListingAPIView",
"RemoveTeamMemberAPIView",
]
36 changes: 32 additions & 4 deletions django/thunderstore/api/cyberstorm/views/team.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from django.core.exceptions import ValidationError as DjangoValidationError
from django.db.models import Q, QuerySet
from rest_framework import status
from rest_framework.exceptions import PermissionDenied, ValidationError
Expand All @@ -17,7 +16,11 @@
CyberstormTeamMemberSerializer,
CyberstormTeamSerializer,
)
from thunderstore.api.cyberstorm.services import team as team_services
from thunderstore.api.cyberstorm.services.team import (
create_team,
disband_team,
remove_team_member,
)
from thunderstore.api.ordering import StrictOrderingFilter
from thunderstore.api.utils import (
CyberstormAutoSchemaMixin,
Expand All @@ -27,6 +30,14 @@
from thunderstore.repository.models.team import Team, TeamMember


def get_team_member_object_or_404(team_name: str, team_member: str) -> TeamMember:
return get_object_or_404(

Check warning on line 34 in django/thunderstore/api/cyberstorm/views/team.py

View check run for this annotation

Codecov / codecov/patch

django/thunderstore/api/cyberstorm/views/team.py#L34

Added line #L34 was not covered by tests
TeamMember.objects.real_users(),
team__name=team_name,
user__username=team_member,
)


class TeamPermissionsMixin:
permission_classes = [IsAuthenticated]

Expand Down Expand Up @@ -66,7 +77,7 @@
serializer = CyberstormCreateTeamSerializer(data=request.data)
serializer.is_valid(raise_exception=True)
team_name = serializer.validated_data["name"]
team = team_services.create_team(user=request.user, team_name=team_name)
team = create_team(user=request.user, team_name=team_name)

Check warning on line 80 in django/thunderstore/api/cyberstorm/views/team.py

View check run for this annotation

Codecov / codecov/patch

django/thunderstore/api/cyberstorm/views/team.py#L80

Added line #L80 was not covered by tests
return_data = CyberstormTeamSerializer(team).data
return Response(return_data, status=status.HTTP_201_CREATED)

Expand Down Expand Up @@ -135,5 +146,22 @@
)
def delete(self, request, *args, **kwargs):
team_name = kwargs["team_name"]
team_services.disband_team(user=request.user, team_name=team_name)
disband_team(user=request.user, team_name=team_name)
return Response(status=status.HTTP_204_NO_CONTENT)

Check warning on line 150 in django/thunderstore/api/cyberstorm/views/team.py

View check run for this annotation

Codecov / codecov/patch

django/thunderstore/api/cyberstorm/views/team.py#L149-L150

Added lines #L149 - L150 were not covered by tests


class RemoveTeamMemberAPIView(APIView):
permission_classes = [IsAuthenticated]

@conditional_swagger_auto_schema(
operation_id="cyberstorm.team.member.remove",
tags=["cyberstorm"],
responses={status.HTTP_204_NO_CONTENT: ""},
)
def delete(self, request, *args, **kwargs):
team_member = get_team_member_object_or_404(

Check warning on line 162 in django/thunderstore/api/cyberstorm/views/team.py

View check run for this annotation

Codecov / codecov/patch

django/thunderstore/api/cyberstorm/views/team.py#L162

Added line #L162 was not covered by tests
team_name=kwargs["team_name"],
team_member=kwargs["team_member"],
)
remove_team_member(agent=request.user, team_member=team_member)

Check warning on line 166 in django/thunderstore/api/cyberstorm/views/team.py

View check run for this annotation

Codecov / codecov/patch

django/thunderstore/api/cyberstorm/views/team.py#L166

Added line #L166 was not covered by tests
return Response(status=status.HTTP_204_NO_CONTENT)
Loading
Loading