diff --git a/apps/api/plane/api/serializers/__init__.py b/apps/api/plane/api/serializers/__init__.py index d0278eb1415..4e1745eeaa3 100644 --- a/apps/api/plane/api/serializers/__init__.py +++ b/apps/api/plane/api/serializers/__init__.py @@ -26,6 +26,7 @@ IssueLinkCreateSerializer, IssueLinkUpdateSerializer, IssueRelationCreateSerializer, + IssueRelationRemoveSerializer, IssueRelationResponseSerializer, IssueRelationSerializer, RelatedIssueSerializer, diff --git a/apps/api/plane/api/urls/work_item.py b/apps/api/plane/api/urls/work_item.py index 1a1704f2773..df228b202d5 100644 --- a/apps/api/plane/api/urls/work_item.py +++ b/apps/api/plane/api/urls/work_item.py @@ -18,6 +18,7 @@ WorkspaceIssueAPIEndpoint, IssueSearchEndpoint, IssueRelationListCreateAPIEndpoint, + IssueRelationRemoveAPIEndpoint, ) # Deprecated url patterns @@ -151,6 +152,11 @@ IssueRelationListCreateAPIEndpoint.as_view(http_method_names=["get", "post"]), name="work-item-relation-list", ), + path( + "workspaces//projects//work-items//relations/remove/", + IssueRelationRemoveAPIEndpoint.as_view(http_method_names=["post"]), + name="work-item-relation-remove", + ), ] urlpatterns = old_url_patterns + new_url_patterns diff --git a/apps/api/plane/api/views/__init__.py b/apps/api/plane/api/views/__init__.py index 5e4660a7b2b..ee35d14c531 100644 --- a/apps/api/plane/api/views/__init__.py +++ b/apps/api/plane/api/views/__init__.py @@ -31,6 +31,7 @@ IssueAttachmentDetailAPIEndpoint, IssueSearchEndpoint, IssueRelationListCreateAPIEndpoint, + IssueRelationRemoveAPIEndpoint, ) from .cycle import ( diff --git a/apps/api/plane/api/views/issue.py b/apps/api/plane/api/views/issue.py index da9edc66d66..5a6b91c37c8 100644 --- a/apps/api/plane/api/views/issue.py +++ b/apps/api/plane/api/views/issue.py @@ -47,6 +47,7 @@ IssueCommentSerializer, IssueLinkSerializer, IssueRelationCreateSerializer, + IssueRelationRemoveSerializer, IssueRelationResponseSerializer, IssueRelationSerializer, IssueSerializer, @@ -88,7 +89,7 @@ from plane.bgtasks.storage_metadata_task import get_asset_object_metadata from .base import BaseAPIView from plane.utils.host import base_host -from plane.utils.issue_relation_mapper import get_actual_relation +from plane.utils.issue_relation_mapper import get_actual_relation, get_inverse_relation from plane.bgtasks.webhook_task import model_activity from plane.app.permissions import ROLE from plane.utils.openapi import ( @@ -2587,3 +2588,96 @@ def post(self, request, slug, project_id, issue_id): serializer_class(refetched_relations, many=True).data, status=status.HTTP_201_CREATED, ) + + +class IssueRelationRemoveAPIEndpoint(BaseAPIView): + """Issue Relation Remove Endpoint""" + + serializer_class = IssueRelationRemoveSerializer + model = IssueRelation + permission_classes = [ProjectEntityPermission] + + @work_item_relation_docs( + operation_id="remove_work_item_relation", + summary="Remove work item relation", + description="Remove an existing relationship between two work items. The relation is matched in either direction, so the same request works whether it was created from this work item or from the related one.", # noqa E501 + parameters=[ + ISSUE_ID_PARAMETER, + ], + request=OpenApiRequest( + request=IssueRelationRemoveSerializer, + examples=[ + OpenApiExample( + name="Remove relation", + value={"related_issue": "550e8400-e29b-41d4-a716-446655440000"}, + ) + ], + ), + responses={ + 204: OpenApiResponse(description="Work item relation removed successfully"), + 400: INVALID_REQUEST_RESPONSE, + 404: ISSUE_NOT_FOUND_RESPONSE, + }, + ) + def post(self, request, slug, project_id, issue_id): + """Remove work item relation + + Remove the relation between a work item and a related work item. + Automatically tracks relation removal activity for both work items. + """ + # Validate request data using serializer + serializer = IssueRelationRemoveSerializer(data=request.data) + if not serializer.is_valid(): + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + related_issue_id = serializer.validated_data["related_issue"] + + # The work item has to live in the project the request is scoped to, + # otherwise membership of that project would not authorize the removal. + if not Issue.objects.filter(pk=issue_id, project_id=project_id, workspace__slug=slug).exists(): + return Response({"error": "Work item not found"}, status=status.HTTP_404_NOT_FOUND) + + # Relations can cross projects so only workspace scope is enforced. + # The pair is matched in both directions since either work item may be + # the source of the stored relation. + issue_relation = ( + IssueRelation.objects.filter( + Q(issue_id=issue_id, related_issue_id=related_issue_id) + | Q(issue_id=related_issue_id, related_issue_id=issue_id), + workspace__slug=slug, + ) + .select_related("related_issue__state") + .first() + ) + + if issue_relation is None: + return Response( + {"error": "Work item relation not found"}, + status=status.HTTP_404_NOT_FOUND, + ) + + # Stored relations are directional. Report the type as seen from the + # work item in the path so the activity feed reads the right way round. + relation_type = issue_relation.relation_type + if str(issue_relation.related_issue_id) == str(issue_id): + relation_type = get_inverse_relation(relation_type) + + current_instance = json.dumps(IssueRelationSerializer(issue_relation).data, cls=DjangoJSONEncoder) + issue_relation.delete() + + issue_activity.delay( + type="issue_relation.activity.deleted", + requested_data=json.dumps( + {"related_issue": str(related_issue_id), "relation_type": relation_type}, + cls=DjangoJSONEncoder, + ), + actor_id=str(request.user.id), + issue_id=str(issue_id), + project_id=str(project_id), + current_instance=current_instance, + epoch=int(timezone.now().timestamp()), + notification=True, + origin=base_host(request=request, is_app=True), + ) + + return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/apps/api/plane/app/views/issue/relation.py b/apps/api/plane/app/views/issue/relation.py index 5fe44e55f2d..e12a6bcafcf 100644 --- a/apps/api/plane/app/views/issue/relation.py +++ b/apps/api/plane/app/views/issue/relation.py @@ -277,6 +277,11 @@ def remove_relation(self, request, slug, project_id, issue_id): Q(issue_id=related_issue, related_issue_id=issue_id) | Q(issue_id=issue_id, related_issue_id=related_issue) ) issue_relations = issue_relations.first() + if issue_relations is None: + return Response( + {"error": "Work item relation not found"}, + status=status.HTTP_404_NOT_FOUND, + ) current_instance = json.dumps(IssueRelationSerializer(issue_relations).data, cls=DjangoJSONEncoder) issue_relations.delete() issue_activity.delay( diff --git a/apps/api/plane/tests/conftest.py b/apps/api/plane/tests/conftest.py index 870779c42d6..940085be478 100644 --- a/apps/api/plane/tests/conftest.py +++ b/apps/api/plane/tests/conftest.py @@ -3,9 +3,11 @@ # See the LICENSE file for details. import pytest +from django.core.cache import cache from rest_framework.test import APIClient from pytest_django.fixtures import django_db_setup +from plane.api.rate_limit import ApiKeyRateThrottle from plane.db.models import User, Workspace, WorkspaceMember from plane.db.models.api import APIToken @@ -60,6 +62,11 @@ def api_token(db, create_user): @pytest.fixture def api_key_client(api_client, api_token): """Return an API key authenticated client for external API testing""" + # ApiKeyRateThrottle counts requests per token in the shared cache, which + # outlives the test that made them. Every test reuses the same token, so + # the history accumulates until later tests are rate limited into 429s. + # Give each test the full budget. + cache.delete(f"{ApiKeyRateThrottle.scope}:{api_token.token}") api_client.credentials(HTTP_X_API_KEY=api_token.token) return api_client diff --git a/apps/api/plane/tests/contract/api/test_work_item_relations.py b/apps/api/plane/tests/contract/api/test_work_item_relations.py new file mode 100644 index 00000000000..a21c6756d14 --- /dev/null +++ b/apps/api/plane/tests/contract/api/test_work_item_relations.py @@ -0,0 +1,266 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +from unittest.mock import patch + +import pytest +from rest_framework import status + +from plane.db.models import Issue, IssueRelation, Project, ProjectMember, State + + +def _make_project(workspace, create_user, name, identifier): + """Create a project with the requesting user as an active admin member.""" + project = Project.objects.create( + name=name, + identifier=identifier, + workspace=workspace, + created_by=create_user, + ) + ProjectMember.objects.create( + project=project, + member=create_user, + role=20, # Admin role + is_active=True, + ) + # A default state is required for work items created in the project + State.objects.create( + name="Backlog", + color="#000000", + group="backlog", + default=True, + project=project, + workspace=workspace, + created_by=create_user, + ) + return project + + +@pytest.fixture +def project(db, workspace, create_user): + return _make_project(workspace, create_user, "Test Project", "TP") + + +@pytest.fixture +def other_project(db, workspace, create_user): + """A second project in the same workspace, so relations can cross projects.""" + return _make_project(workspace, create_user, "Other Project", "OP") + + +@pytest.fixture +def issue(db, workspace, project, create_user): + return Issue.objects.create( + name="Blocked Issue", + project=project, + workspace=workspace, + created_by=create_user, + ) + + +@pytest.fixture +def related_issue(db, workspace, project, create_user): + return Issue.objects.create( + name="Blocking Issue", + project=project, + workspace=workspace, + created_by=create_user, + ) + + +@pytest.mark.contract +class TestWorkItemRelationRemoveContract: + """ + Contract: the documented relation removal endpoint + + ``POST /api/v1/workspaces/{slug}/projects/{project_id}/work-items/{issue_id}/relations/remove/`` + + exists on the external REST API, so relations created through the API can + also be removed through it. See makeplane/plane#9584. + """ + + def get_remove_url(self, workspace_slug, project_id, issue_id): + """Helper to build the relation removal endpoint URL.""" + return f"/api/v1/workspaces/{workspace_slug}/projects/{project_id}/work-items/{issue_id}/relations/remove/" + + def get_list_url(self, workspace_slug, project_id, issue_id): + """Helper to build the relation list/create endpoint URL.""" + return f"/api/v1/workspaces/{workspace_slug}/projects/{project_id}/work-items/{issue_id}/relations/" + + @pytest.mark.django_db + def test_remove_relation_returns_204_and_deletes_the_relation( + self, api_key_client, workspace, project, issue, related_issue + ): + """The relation stored from the work item in the path is removed.""" + IssueRelation.objects.create( + issue=issue, + related_issue=related_issue, + relation_type="blocked_by", + project=project, + workspace=workspace, + ) + url = self.get_remove_url(workspace.slug, project.id, issue.id) + + response = api_key_client.post(url, {"related_issue": str(related_issue.id)}, format="json") + + assert response.status_code == status.HTTP_204_NO_CONTENT, f"Got {response.status_code}: {response.data!r}" + assert not IssueRelation.objects.filter(issue=issue, related_issue=related_issue).exists() + + @pytest.mark.django_db + def test_remove_relation_matches_the_reverse_direction( + self, api_key_client, workspace, project, issue, related_issue + ): + """A relation stored the other way round is removable from either side. + + ``blocked_by`` is stored once, so the work item on the ``blocking`` side + has to match on ``related_issue_id`` instead of ``issue_id``. + """ + IssueRelation.objects.create( + issue=related_issue, + related_issue=issue, + relation_type="blocked_by", + project=project, + workspace=workspace, + ) + url = self.get_remove_url(workspace.slug, project.id, issue.id) + + response = api_key_client.post(url, {"related_issue": str(related_issue.id)}, format="json") + + assert response.status_code == status.HTTP_204_NO_CONTENT, f"Got {response.status_code}: {response.data!r}" + assert not IssueRelation.objects.filter(issue=related_issue, related_issue=issue).exists() + + @pytest.mark.django_db + def test_remove_relation_across_projects( + self, api_key_client, workspace, project, other_project, issue, create_user + ): + """Relations may cross projects, so removal is scoped to the workspace.""" + cross_project_issue = Issue.objects.create( + name="Cross Project Issue", + project=other_project, + workspace=workspace, + created_by=create_user, + ) + IssueRelation.objects.create( + issue=issue, + related_issue=cross_project_issue, + relation_type="relates_to", + project=project, + workspace=workspace, + ) + url = self.get_remove_url(workspace.slug, project.id, issue.id) + + response = api_key_client.post(url, {"related_issue": str(cross_project_issue.id)}, format="json") + + assert response.status_code == status.HTTP_204_NO_CONTENT, f"Got {response.status_code}: {response.data!r}" + assert not IssueRelation.objects.filter(issue=issue, related_issue=cross_project_issue).exists() + + @pytest.mark.django_db + def test_remove_missing_relation_returns_404(self, api_key_client, workspace, project, issue, related_issue): + """No matching relation is a 404, not an unhandled AttributeError → 500.""" + url = self.get_remove_url(workspace.slug, project.id, issue.id) + + response = api_key_client.post(url, {"related_issue": str(related_issue.id)}, format="json") + + assert response.status_code == status.HTTP_404_NOT_FOUND, f"Got {response.status_code}: {response.data!r}" + + @pytest.mark.django_db + def test_remove_relation_for_work_item_outside_the_project_returns_404( + self, api_key_client, workspace, project, other_project, related_issue, create_user + ): + """The work item in the path has to belong to the project in the path. + + Otherwise membership of the path project would authorize removing + relations of work items in projects the caller cannot see. + """ + foreign_issue = Issue.objects.create( + name="Foreign Issue", + project=other_project, + workspace=workspace, + created_by=create_user, + ) + IssueRelation.objects.create( + issue=foreign_issue, + related_issue=related_issue, + relation_type="blocked_by", + project=other_project, + workspace=workspace, + ) + url = self.get_remove_url(workspace.slug, project.id, foreign_issue.id) + + response = api_key_client.post(url, {"related_issue": str(related_issue.id)}, format="json") + + assert response.status_code == status.HTTP_404_NOT_FOUND, f"Got {response.status_code}: {response.data!r}" + assert IssueRelation.objects.filter(issue=foreign_issue, related_issue=related_issue).exists() + + @pytest.mark.django_db + def test_remove_relation_without_related_issue_returns_400(self, api_key_client, workspace, project, issue): + """``related_issue`` is required.""" + url = self.get_remove_url(workspace.slug, project.id, issue.id) + + response = api_key_client.post(url, {}, format="json") + + assert response.status_code == status.HTTP_400_BAD_REQUEST, f"Got {response.status_code}: {response.data!r}" + assert "related_issue" in response.data + + @pytest.mark.django_db + def test_remove_relation_with_malformed_related_issue_returns_400(self, api_key_client, workspace, project, issue): + """A non-UUID ``related_issue`` is rejected before the database is touched.""" + url = self.get_remove_url(workspace.slug, project.id, issue.id) + + response = api_key_client.post(url, {"related_issue": "not-a-uuid"}, format="json") + + assert response.status_code == status.HTTP_400_BAD_REQUEST, f"Got {response.status_code}: {response.data!r}" + + @pytest.mark.django_db + def test_remove_relation_dispatches_deletion_activity( + self, api_key_client, workspace, project, issue, related_issue + ): + """Removal records activity for both work items, like the web app does.""" + IssueRelation.objects.create( + issue=related_issue, + related_issue=issue, + relation_type="blocked_by", + project=project, + workspace=workspace, + ) + url = self.get_remove_url(workspace.slug, project.id, issue.id) + + with patch("plane.api.views.issue.issue_activity") as mock_issue_activity: + response = api_key_client.post(url, {"related_issue": str(related_issue.id)}, format="json") + + assert response.status_code == status.HTTP_204_NO_CONTENT, f"Got {response.status_code}: {response.data!r}" + + mock_issue_activity.delay.assert_called_once() + kwargs = mock_issue_activity.delay.call_args.kwargs + assert kwargs["type"] == "issue_relation.activity.deleted" + assert kwargs["notification"] is True + # The activity feed needs the relation type as seen from the work item + # in the path, which is the inverse of how this one is stored. + assert '"relation_type": "blocking"' in kwargs["requested_data"] + + @pytest.mark.django_db + def test_relation_round_trip_through_the_public_api(self, api_key_client, workspace, project, issue, related_issue): + """Create, read and remove a relation using only documented endpoints.""" + list_url = self.get_list_url(workspace.slug, project.id, issue.id) + + create_response = api_key_client.post( + list_url, + {"relation_type": "blocked_by", "issues": [str(related_issue.id)]}, + format="json", + ) + assert create_response.status_code == status.HTTP_201_CREATED + + read_response = api_key_client.get(list_url) + assert read_response.status_code == status.HTTP_200_OK + assert [ref["issue_id"] for ref in read_response.data["blocked_by"]] == [str(related_issue.id)] + + remove_response = api_key_client.post( + self.get_remove_url(workspace.slug, project.id, issue.id), + {"related_issue": str(related_issue.id)}, + format="json", + ) + assert remove_response.status_code == status.HTTP_204_NO_CONTENT + + read_response = api_key_client.get(list_url) + assert read_response.status_code == status.HTTP_200_OK + assert read_response.data["blocked_by"] == []