Skip to content
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

feat(tags): prevent system tag modifications #3605

Merged
merged 3 commits into from
Mar 13, 2024
Merged
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
25 changes: 18 additions & 7 deletions api/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,20 @@ def chargebee_subscription(organisation: Organisation) -> Subscription:


@pytest.fixture()
def project(organisation):
return Project.objects.create(name="Test Project", organisation=organisation)
def tag(project):
return Tag.objects.create(label="tag", project=project, color="#000000")


@pytest.fixture()
def tag(project):
return Tag.objects.create(label="tag", project=project, color="#000000")
def system_tag(project: Project) -> Tag:
return Tag.objects.create(
label="system-tag", project=project, color="#FFFFFF", is_system_tag=True
)


@pytest.fixture()
def project(organisation):
return Project.objects.create(name="Test Project", organisation=organisation)


@pytest.fixture()
Expand Down Expand Up @@ -229,13 +236,17 @@ def with_project_permissions(
"""

def _with_project_permissions(
permission_keys: list[str], project_id: typing.Optional[int] = None
permission_keys: list[str] = None,
project_id: typing.Optional[int] = None,
admin: bool = False,
) -> UserProjectPermission:
project_id = project_id or project.id
upp, __ = UserProjectPermission.objects.get_or_create(
project_id=project_id, user=staff_user
project_id=project_id, user=staff_user, admin=admin
)
upp.permissions.add(*permission_keys)

if permission_keys:
upp.permissions.add(*permission_keys)

return upp

Expand Down
7 changes: 7 additions & 0 deletions api/projects/tags/serializers.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from typing import Any

from rest_framework import serializers

from projects.tags.models import Tag
Expand All @@ -18,3 +20,8 @@ class Meta:
"type",
)
read_only_fields = ("project", "uuid", "is_system_tag", "type")

def validate(self, attrs: dict[str, Any]) -> dict[str, Any]:
if self.instance and self.instance.is_system_tag:
raise serializers.ValidationError("Cannot update a system tag.")
return super().validate(attrs)
14 changes: 13 additions & 1 deletion api/projects/tags/views.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from rest_framework import viewsets
from rest_framework import status, viewsets
from rest_framework.decorators import action
from rest_framework.generics import get_object_or_404
from rest_framework.permissions import IsAuthenticated
Expand Down Expand Up @@ -36,6 +36,18 @@ def perform_update(self, serializer):
project_id = int(self.kwargs["project_pk"])
serializer.save(project_id=project_id)

def destroy(self, request: Request, *args, **kwargs):
instance = self.get_object()

if instance.is_system_tag:
return Response(
status=status.HTTP_400_BAD_REQUEST,
data={"message": "Cannot delete a system tag."},
)

self.perform_destroy(instance)
return Response(status=status.HTTP_204_NO_CONTENT)

@action(
detail=False,
url_path=r"get-by-uuid/(?P<uuid>[0-9a-f-]+)",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import json

import pytest
from django.urls import reverse
from pytest_lazyfixture import lazy_fixture
Expand Down Expand Up @@ -68,3 +70,51 @@ def test_get_tag_by__uuid_returns_200_for_user_with_view_project_permission(
# Then
assert response.status_code == status.HTTP_200_OK
assert response.json()["uuid"] == str(tag.uuid)


def test_cannot_delete_a_system_tag(
staff_client: APIClient,
with_project_permissions: WithProjectPermissionsCallable,
project: Project,
system_tag: Tag,
) -> None:
# Given
url = reverse("api-v1:projects:tags-detail", args=(project.id, system_tag.id))

with_project_permissions(admin=True)

# When
response = staff_client.delete(url)

# Then
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.json() == {"message": "Cannot delete a system tag."}


def test_cannot_update_a_system_tag(
staff_client: APIClient,
with_project_permissions: WithProjectPermissionsCallable,
project: Project,
system_tag: Tag,
):
# Given
url = reverse("api-v1:projects:tags-detail", args=(project.id, system_tag.id))

with_project_permissions(admin=True)

updated_label = f"{system_tag.label} updated"

data = {
"id": system_tag.id,
"color": system_tag.color,
"label": updated_label,
"project": system_tag.project_id,
"description": "",
}

# When
response = staff_client.put(url, json.dumps(data), content_type="application/json")

# Then
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.json() == {"non_field_errors": ["Cannot update a system tag."]}
Loading