diff --git a/django/thunderstore/api/cyberstorm/services/team.py b/django/thunderstore/api/cyberstorm/services/team.py index 45d282f50..bcd732656 100644 --- a/django/thunderstore/api/cyberstorm/services/team.py +++ b/django/thunderstore/api/cyberstorm/services/team.py @@ -1,36 +1,28 @@ -from django.core.exceptions import ValidationError from django.db import transaction -from django.shortcuts import get_object_or_404 from thunderstore.account.models import ServiceAccount from thunderstore.core.exceptions import PermissionValidationError from thunderstore.core.types import UserType -from thunderstore.repository.models import Namespace, Team, TeamMember +from thunderstore.repository.models import Team, TeamMember from thunderstore.repository.models.team import TeamMemberRole @transaction.atomic -def disband_team(user: UserType, team_name: str) -> None: - teams = Team.objects.exclude(is_active=False) - team = get_object_or_404(teams, name=team_name) - team.ensure_user_can_access(user) - team.ensure_user_can_disband(user) +def disband_team(agent: UserType, team: Team) -> None: + team.ensure_user_can_access(agent) + team.ensure_user_can_disband(agent) team.delete() @transaction.atomic -def create_team(user: UserType, team_name: str) -> Team: - if not user or not user.is_authenticated or not user.is_active: +def create_team(agent: UserType, team_name: str) -> Team: + if not agent or not agent.is_authenticated or not agent.is_active: raise PermissionValidationError("Must be authenticated to create teams") - if getattr(user, "service_account", None) is not None: + if getattr(agent, "service_account", None) is not None: raise PermissionValidationError("Service accounts cannot create teams") - if Team.objects.filter(name=team_name).exists(): - raise ValidationError("A team with the provided name already exists") - if Namespace.objects.filter(name=team_name).exists(): - raise ValidationError("A namespace with the provided name already exists") - team = Team.objects.create(name=team_name) - team.add_member(user=user, role=TeamMemberRole.owner) + team = Team.create(name=team_name) + team.add_member(user=agent, role=TeamMemberRole.owner) return team diff --git a/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py b/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py index 8e10442b2..4304924c5 100644 --- a/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py +++ b/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py @@ -1,6 +1,5 @@ import pytest from django.core.exceptions import ValidationError -from django.http import Http404 from conftest import TestUserTypes from thunderstore.api.cyberstorm.services import team as team_services @@ -13,45 +12,76 @@ @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) + team_services.disband_team(agent=team_owner.user, team=team_owner.team) 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): + team_services.disband_team(agent=user, team=team) @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): + team_services.disband_team(agent=team_member.user, team=team_member.team) + + +@pytest.mark.django_db +def test_disband_team_with_packages(package, team_owner): + team = team_owner.team + package.owner = team + package.save() + + with pytest.raises(ValidationError, match="Unable to disband teams with packages"): + team_services.disband_team(agent=team_owner.user, team=team) + + +@pytest.mark.django_db +def test_disband_team_user_is_service_account(service_account, team): + service_account_user = service_account.user + error_msg = "Service accounts are unable to perform this action" + with pytest.raises(PermissionValidationError, match=error_msg): + team_services.disband_team(agent=service_account_user, team=team) + + +@pytest.mark.django_db +def test_disband_team_user_not_authenticated(team): + error_msg = "Must be authenticated" + with pytest.raises(PermissionValidationError, match=error_msg): + team_services.disband_team(agent=None, team=team) + + +@pytest.mark.django_db +def test_disband_team_user_not_active(user, team): + user.is_active = False + user.save() + + error_msg = "User has been deactivated" + with pytest.raises(PermissionValidationError, match=error_msg): + team_services.disband_team(agent=user, team=team) @pytest.mark.django_db def test_create_team_name_exists_in_team(user): Team.objects.create(name="existing_team") - error_msg = "A team with the provided name already exists" + error_msg = "Team with this name already exists" with pytest.raises(ValidationError, match=error_msg): - team_services.create_team(user, "existing_team") + team_services.create_team(agent=user, team_name="existing_team") @pytest.mark.django_db def test_create_team_name_exists_in_namespace(user): Namespace.objects.create(name="existing_namespace") - error_msg = "A namespace with the provided name already exists" + error_msg = "Namespace with this name already exists" with pytest.raises(ValidationError, match=error_msg): - team_services.create_team(user, "existing_namespace") + team_services.create_team(agent=user, team_name="existing_namespace") @pytest.mark.django_db @@ -59,14 +89,14 @@ 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): + team_services.create_team(agent=service_account_user, team_name="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 = team_services.create_team(agent=user, team_name=team_name) assert Team.objects.filter(name=team_name).exists() assert team.name == team_name @@ -330,3 +360,40 @@ def test_update_team_member_cannot_remove_last_owner(team_owner): team_services.update_team_member( team_owner.user, team_owner, TeamMemberRole.member ) + + +@pytest.mark.django_db +def test_create_team_user_not_authenticated(): + error_msg = "Must be authenticated to create teams" + with pytest.raises(PermissionValidationError, match=error_msg): + team_services.create_team(agent=None, team_name="new_team") + + +@pytest.mark.django_db +def test_create_team_user_not_active(user): + user.is_active = False + user.save() + + error_msg = "Must be authenticated to create teams" + with pytest.raises(PermissionValidationError, match=error_msg): + team_services.create_team(agent=user, team_name="new_team") + + +@pytest.mark.django_db +@pytest.mark.parametrize( + ("name1", "name2", "should_fail"), + ( + ("Team", "team", True), + ("Team", "t_eam", False), + ("team", "teaM", True), + ("team", "team", True), + ), +) +def test_create_team_name_conflict(user, name1: str, name2: str, should_fail: bool): + Team.create(name=name1) + if should_fail: + with pytest.raises(ValidationError): + team_services.create_team(agent=user, team_name=name2) + else: + team = team_services.create_team(agent=user, team_name=name2) + assert team.name == name2 diff --git a/django/thunderstore/api/cyberstorm/tests/test_create_team.py b/django/thunderstore/api/cyberstorm/tests/test_create_team.py index 149e1b883..15d1c0847 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_create_team.py +++ b/django/thunderstore/api/cyberstorm/tests/test_create_team.py @@ -53,9 +53,7 @@ def test_create_team__fail_because_team_with_provided_name_exists( ): api_client.force_authenticate(user) response = make_request(api_client, team.name) - expected_response = { - "non_field_errors": ["A team with the provided name already exists"] - } + expected_response = {"non_field_errors": ["Team with this name already exists"]} assert response.status_code == 400 assert response.json() == expected_response @@ -70,7 +68,7 @@ def test_create_team__fail_because_team_with_provided_namespace_exists( NamespaceFactory(name="CoolestTeamNameEver") response = make_request(api_client, "CoolestTeamNameEver") expected_response = { - "non_field_errors": ["A namespace with the provided name already exists"] + "non_field_errors": ["Namespace with this name already exists"] } assert response.status_code == 400 diff --git a/django/thunderstore/api/cyberstorm/views/team.py b/django/thunderstore/api/cyberstorm/views/team.py index 8a5297784..2c7327ddf 100644 --- a/django/thunderstore/api/cyberstorm/views/team.py +++ b/django/thunderstore/api/cyberstorm/views/team.py @@ -75,8 +75,10 @@ class TeamCreateAPIView(APIView): def post(self, request, *args, **kwargs): serializer = CyberstormCreateTeamSerializer(data=request.data) serializer.is_valid(raise_exception=True) + team_name = serializer.validated_data["name"] - team = create_team(user=request.user, team_name=team_name) + team = create_team(agent=request.user, team_name=team_name) + return_data = CyberstormTeamSerializer(team).data return Response(return_data, status=status.HTTP_201_CREATED) @@ -166,8 +168,8 @@ class DisbandTeamAPIView(APIView): responses={status.HTTP_204_NO_CONTENT: ""}, ) def delete(self, request, *args, **kwargs): - team_name = kwargs["team_name"] - disband_team(user=request.user, team_name=team_name) + team = get_object_or_404(Team, name=kwargs["team_name"]) + disband_team(agent=request.user, team=team) return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/django/thunderstore/repository/forms/team.py b/django/thunderstore/repository/forms/team.py index f55066689..6cf5960e3 100644 --- a/django/thunderstore/repository/forms/team.py +++ b/django/thunderstore/repository/forms/team.py @@ -3,21 +3,17 @@ from django import forms from django.contrib.auth import get_user_model from django.core.exceptions import ObjectDoesNotExist, ValidationError +from django.db import transaction from thunderstore.api.cyberstorm.services.team import ( + create_team, + disband_team, remove_team_member, update_team, update_team_member, ) -from thunderstore.core.exceptions import PermissionValidationError from thunderstore.core.types import UserType -from thunderstore.repository.models import ( - Namespace, - Team, - TeamMember, - TeamMemberRole, - transaction, -) +from thunderstore.repository.models import Team, TeamMember, TeamMemberRole from thunderstore.repository.validators import PackageReferenceComponentValidator User = get_user_model() @@ -38,23 +34,22 @@ def __init__(self, user: UserType, *args, **kwargs): def clean_name(self): name = self.cleaned_data["name"] - if Team.objects.filter(name__iexact=name.lower()).exists(): - raise ValidationError(f"A team with the provided name already exists") - if Namespace.objects.filter(name__iexact=name.lower()).exists(): - raise ValidationError("A namespace with the provided name already exists") + if Team.objects.filter(name__iexact=name).exists(): + raise ValidationError("Team with this name already exists") return name - def clean(self): - if not self.user or not self.user.is_authenticated or not self.user.is_active: - raise PermissionValidationError("Must be authenticated to create teams") - if getattr(self.user, "service_account", None) is not None: - raise PermissionValidationError("Service accounts cannot create teams") - return super().clean() - @transaction.atomic def save(self, *args, **kwargs) -> Team: - instance = super().save() - instance.add_member(user=self.user, role=TeamMemberRole.owner) + if self.errors: + raise ValidationError(self.errors) + + try: + team_name = self.cleaned_data["name"] + instance = create_team(agent=self.user, team_name=team_name) + except ValidationError as e: + self.add_error(None, e) + raise ValidationError(self.errors) + return instance @@ -169,13 +164,17 @@ def clean_verification(self): def clean(self): if not self.instance.pk: raise ValidationError("Missing team instance") - self.instance.ensure_user_can_disband(self.user) return super().clean() @transaction.atomic def save(self, **kwargs): - self.instance.ensure_user_can_disband(self.user) - self.instance.delete() + if self.errors: + raise ValidationError(self.errors) + try: + disband_team(agent=self.user, team=self.instance) + except ValidationError as e: + self.add_error(None, e) + raise ValidationError(self.errors) class DonationLinkTeamForm(forms.ModelForm): diff --git a/django/thunderstore/repository/models/team.py b/django/thunderstore/repository/models/team.py index 8bff5acb8..0fbf63794 100644 --- a/django/thunderstore/repository/models/team.py +++ b/django/thunderstore/repository/models/team.py @@ -187,13 +187,9 @@ def add_member(self, user: UserType, role: str) -> TeamMember: @classmethod @transaction.atomic def create(cls, name, **kwargs): - existing_ns = Namespace.objects.filter(name__iexact=name).first() - if existing_ns: - raise ValidationError("Namespace with the Teams name exists") - else: - team = cls.objects.create(name=name, **kwargs) - Namespace.objects.create(name=name, team=team) - return team + team = cls.objects.create(name=name, **kwargs) + Namespace.objects.create(name=name, team=team) + return team @classmethod @transaction.atomic diff --git a/django/thunderstore/repository/tests/test_team.py b/django/thunderstore/repository/tests/test_team.py index 79cb2be82..9548c847e 100644 --- a/django/thunderstore/repository/tests/test_team.py +++ b/django/thunderstore/repository/tests/test_team.py @@ -75,7 +75,7 @@ def test_team_create_namespace_creation() -> None: ns.save() with pytest.raises(ValidationError) as e: Team.create(name="taken_namespace") - assert "Namespace with the Teams name exists" in str(e.value) + assert "Namespace with this name already exists" in str(e.value) @pytest.mark.django_db @@ -769,3 +769,39 @@ def test_team_ensure_user_can_manage_packages( else: assert team.can_user_manage_packages(user) is True assert team.ensure_user_can_manage_packages(user) is None + + +@pytest.mark.django_db +def test_team_create__success(): + Team.create(name="TestTeam") + assert Team.objects.filter(name="TestTeam").count() == 1 + assert Namespace.objects.filter(name="TestTeam").count() == 1 + + +@pytest.mark.django_db +def test_team_create__team_exists_fail(team): + with pytest.raises(ValidationError) as e: + Team.create(name=team.name) + assert "Team with this name already exists" in str(e.value) + assert Team.objects.filter(name=team.name).count() == 1 + assert Namespace.objects.filter(name=team.name).count() == 1 + + +@pytest.mark.django_db +def test_team_create__namespace_exists_fail(): + NamespaceFactory.create(name="TestTeam") + with pytest.raises(ValidationError) as e: + Team.create(name="TestTeam") + assert "Namespace with this name already exists" in str(e.value) + assert Team.objects.filter(name="TestTeam").count() == 0 + assert Namespace.objects.filter(name="TestTeam").count() == 1 + + +@pytest.mark.django_db +def test_team_create__team_name_read_only_fail(team): + with pytest.raises(ValidationError) as e: + team.name = "NewName" + team.save() + assert "Team name is read only" in str(e.value) + team.refresh_from_db() + assert team.name != "NewName" diff --git a/django/thunderstore/repository/tests/test_team_forms.py b/django/thunderstore/repository/tests/test_team_forms.py index 3771c0597..840067a9a 100644 --- a/django/thunderstore/repository/tests/test_team_forms.py +++ b/django/thunderstore/repository/tests/test_team_forms.py @@ -40,6 +40,8 @@ def test_form_create_team_valid_data(user_type: str) -> None: data={"name": "TeamName"}, ) if expected_error: + with pytest.raises(ValidationError): + form.save() assert form.is_valid() is False assert expected_error in str(repr(form.errors)) else: @@ -71,8 +73,10 @@ def test_form_create_team_team_name_conflict( data={"name": name2}, ) if should_fail: + with pytest.raises(ValidationError): + form.save() assert form.is_valid() is False - assert "A team with the provided name already exists" in str(repr(form.errors)) + assert "Team with this name already exists" in str(repr(form.errors)) else: assert form.is_valid() is True team = form.save() @@ -461,6 +465,8 @@ def test_form_disband_team_form( assert form.save() is None assert Team.objects.filter(pk=team.pk).exists() is False else: + with pytest.raises(ValidationError): + form.save() assert form.is_valid() is False assert form.errors @@ -497,6 +503,8 @@ def test_form_disband_team_form_packages_exist( instance=team, data={"verification": team.name}, ) + with pytest.raises(ValidationError): + form.save() assert form.is_valid() is False assert "Unable to disband teams with packages" in str(repr(form.errors)) diff --git a/django/thunderstore/repository/views/team_settings.py b/django/thunderstore/repository/views/team_settings.py index 1dc83c3be..9f8f5adea 100644 --- a/django/thunderstore/repository/views/team_settings.py +++ b/django/thunderstore/repository/views/team_settings.py @@ -153,7 +153,10 @@ def get_context_data(self, **kwargs): @transaction.atomic def form_valid(self, form): - instance = form.save() + try: + instance = form.save() + except ValidationError: + return super().form_invalid(form) return redirect(instance.settings_url) @@ -175,7 +178,10 @@ def get_form_kwargs(self): @transaction.atomic def form_valid(self, form): - form.save() + try: + form.save() + except ValidationError: + return self.form_invalid(form) return redirect(reverse("settings.teams"))