Skip to content

Commit

Permalink
Use delete() instead of post() in DeleteServiceAccountAPIView
Browse files Browse the repository at this point in the history
- Inherit DestroyAPIView in DeleteServiceAccountAPIView.
- Use delete() instead of post() for deleting service accounts.
- Override destroy() to handle deletion in DeleteServiceAccountAPIView.
- Update tests accordingly.
- Update tests to check the error messages as well.
- Add test for checking uuid validation.

Refs. TS-2320
  • Loading branch information
Roffenlund committed Feb 18, 2025
1 parent 07f929e commit 9e7246d
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def get_delete_service_account_url(team_name: str) -> str:


def make_request(api_client: APIClient, team_name: str, account: ServiceAccount):
return api_client.post(
return api_client.delete(
path=get_delete_service_account_url(team_name),
data=json.dumps({"uuid": str(account.uuid)}),
content_type="application/json",
Expand Down Expand Up @@ -46,8 +46,10 @@ def test_delete_service_account_fail_user_is_not_authenticated(
assert ServiceAccount.objects.filter(uuid=service_account.uuid).count() == 1

response = make_request(api_client, team.name, service_account)
assert response.status_code == 401
expected_response = {"detail": "Authentication credentials were not provided."}

assert response.status_code == 401
assert response.json() == expected_response
assert ServiceAccount.objects.filter(uuid=service_account.uuid).count() == 1


Expand All @@ -63,8 +65,10 @@ def test_delete_service_account_fails_because_user_is_not_team_member(
api_client.force_authenticate(non_team_user)

response = make_request(api_client, team.name, service_account)
assert response.status_code == 403
expected_response = {"detail": "User does not have permission to access this team."}

assert response.status_code == 403
assert response.json() == expected_response
assert ServiceAccount.objects.filter(uuid=service_account.uuid).count() == 1


Expand All @@ -78,8 +82,31 @@ def test_delete_service_account_fail_because_user_is_not_team_owner(
assert ServiceAccount.objects.filter(uuid=service_account.uuid).count() == 1

api_client.force_authenticate(team_member.user)

response = make_request(api_client, team.name, service_account)

error_message = (
"User does not have permission to delete service accounts for this team."
)
expected_response = {"detail": error_message}

assert response.status_code == 403
assert response.json() == expected_response
assert ServiceAccount.objects.filter(uuid=service_account.uuid).count() == 1


@pytest.mark.django_db
def test_delete_service_account_fail_because_invalid_uuid(
api_client: APIClient,
team_owner: TeamMember,
team: Team,
service_account: ServiceAccount,
):
assert ServiceAccount.objects.filter(uuid=service_account.uuid).count() == 1

api_client.force_authenticate(team_owner.user)
response = make_request(api_client, team.name, ServiceAccount(uuid="invalid-uuid"))
expected_response = {"uuid": ["Must be a valid UUID."]}

assert response.status_code == 400
assert response.json() == expected_response
assert ServiceAccount.objects.filter(uuid=service_account.uuid).count() == 1
16 changes: 7 additions & 9 deletions django/thunderstore/api/cyberstorm/views/service_account.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from django.http import HttpRequest
from rest_framework import status
from rest_framework.exceptions import PermissionDenied
from rest_framework.generics import CreateAPIView, GenericAPIView, get_object_or_404
from rest_framework.generics import CreateAPIView, DestroyAPIView, get_object_or_404
from rest_framework.permissions import IsAuthenticated
from rest_framework.response import Response

Expand Down Expand Up @@ -69,7 +69,7 @@ def post(self, request, *args, **kwargs) -> Response:
return super().post(request, *args, **kwargs)


class DeleteServiceAccountAPIView(TeamPermissionMixin, GenericAPIView):
class DeleteServiceAccountAPIView(TeamPermissionMixin, DestroyAPIView):
queryset = ServiceAccount.objects.all()
serializer_class = DeleteServiceAccountSerializer

Expand All @@ -90,14 +90,12 @@ def get_object(self, uuid: str) -> ServiceAccount:
)
return obj

def perform_delete(self, request, *args, **kwargs) -> Response:
def destroy(self, request, *args, **kwargs):
serializer = self.get_serializer(data=request.data)
serializer.is_valid(raise_exception=True)
uuid = serializer.validated_data["uuid"]

service_account = self.get_object(uuid=uuid)
service_account.delete()

instance = self.get_object(uuid=uuid)
instance.delete()
return Response(status=status.HTTP_204_NO_CONTENT)

@conditional_swagger_auto_schema(
Expand All @@ -106,5 +104,5 @@ def perform_delete(self, request, *args, **kwargs) -> Response:
operation_id="cyberstorm.team.service-account.delete",
tags=["cyberstorm"],
)
def post(self, request, *args, **kwargs) -> Response:
return self.perform_delete(request, *args, **kwargs)
def delete(self, request, *args, **kwargs) -> Response:
return self.destroy(request, *args, **kwargs)

0 comments on commit 9e7246d

Please sign in to comment.