From 0b15a32ec606869a5d70d8a4ef98feba74be5a16 Mon Sep 17 00:00:00 2001 From: Nikhil <118773738+pablohashescobar@users.noreply.github.com> Date: Tue, 7 Oct 2025 18:50:09 +0530 Subject: [PATCH] [WEB-5038] fix: cycle creation in external api endpoint (#7866) * feat: set default owner for cycle creation if not provided * Updated CycleListCreateAPIEndpoint to assign the current user as the owner when the 'owned_by' field is not included in the request data. * Enhanced the CycleCreateSerializer initialization to ensure proper ownership assignment during cycle creation. * feat: add comprehensive tests for Cycle API endpoints * Introduced a new test suite for Cycle API endpoints, covering creation, retrieval, updating, and deletion of cycles. * Implemented tests for various scenarios including successful operations, invalid data handling, and conflict resolution with external IDs. * Enhanced test coverage for listing cycles with different view filters and verifying cycle metrics annotations. * feat: enhance CycleCreateSerializer to include ownership assignment * Added 'owned_by' field to CycleCreateSerializer to specify the user who owns the cycle. * Updated CycleListCreateAPIEndpoint to remove redundant ownership assignment logic, relying on the serializer to handle default ownership. * Ensured that if 'owned_by' is not provided, it defaults to the current user during cycle creation. * fix: correct assertion syntax in CycleListCreateAPIEndpoint tests * Updated the assertion in the test for successful cycle creation to use the correct syntax for checking the response status code. * Ensured that the test accurately verifies the expected behavior of the API endpoint. --- apps/api/plane/api/serializers/cycle.py | 13 +- apps/api/plane/api/views/cycle.py | 7 +- .../plane/tests/contract/api/test_cycles.py | 382 ++++++++++++++++++ 3 files changed, 398 insertions(+), 4 deletions(-) create mode 100644 apps/api/plane/tests/contract/api/test_cycles.py diff --git a/apps/api/plane/api/serializers/cycle.py b/apps/api/plane/api/serializers/cycle.py index 726715db7e..6b7bfa442f 100644 --- a/apps/api/plane/api/serializers/cycle.py +++ b/apps/api/plane/api/serializers/cycle.py @@ -4,7 +4,7 @@ from rest_framework import serializers # Module imports from .base import BaseSerializer -from plane.db.models import Cycle, CycleIssue +from plane.db.models import Cycle, CycleIssue, User from plane.utils.timezone_converter import convert_to_utc @@ -16,6 +16,13 @@ class CycleCreateSerializer(BaseSerializer): and UTC normalization for time-bound iteration planning and sprint management. """ + owned_by = serializers.PrimaryKeyRelatedField( + queryset=User.objects.all(), + required=False, + allow_null=True, + help_text="User who owns the cycle. If not provided, defaults to the current user.", + ) + def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) project = self.context.get("project") @@ -72,6 +79,10 @@ class CycleCreateSerializer(BaseSerializer): date=str(data.get("end_date", None).date()), project_id=project_id, ) + + if not data.get("owned_by"): + data["owned_by"] = self.context["request"].user + return data diff --git a/apps/api/plane/api/views/cycle.py b/apps/api/plane/api/views/cycle.py index 17e8b6e665..1908ceada4 100644 --- a/apps/api/plane/api/views/cycle.py +++ b/apps/api/plane/api/views/cycle.py @@ -307,7 +307,8 @@ class CycleListCreateAPIEndpoint(BaseAPIView): if (request.data.get("start_date", None) is None and request.data.get("end_date", None) is None) or ( request.data.get("start_date", None) is not None and request.data.get("end_date", None) is not None ): - serializer = CycleCreateSerializer(data=request.data) + + serializer = CycleCreateSerializer(data=request.data, context={"request": request}) if serializer.is_valid(): if ( request.data.get("external_id") @@ -332,7 +333,7 @@ class CycleListCreateAPIEndpoint(BaseAPIView): }, status=status.HTTP_409_CONFLICT, ) - serializer.save(project_id=project_id, owned_by=request.user) + serializer.save(project_id=project_id) # Send the model activity model_activity.delay( model_name="cycle", @@ -518,7 +519,7 @@ class CycleDetailAPIEndpoint(BaseAPIView): status=status.HTTP_400_BAD_REQUEST, ) - serializer = CycleUpdateSerializer(cycle, data=request.data, partial=True) + serializer = CycleUpdateSerializer(cycle, data=request.data, partial=True, context={"request": request}) if serializer.is_valid(): if ( request.data.get("external_id") diff --git a/apps/api/plane/tests/contract/api/test_cycles.py b/apps/api/plane/tests/contract/api/test_cycles.py new file mode 100644 index 0000000000..fb4ad3f335 --- /dev/null +++ b/apps/api/plane/tests/contract/api/test_cycles.py @@ -0,0 +1,382 @@ +import pytest +from rest_framework import status +from django.db import IntegrityError +from django.utils import timezone +from datetime import datetime, timedelta +from uuid import uuid4 + +from plane.db.models import Cycle, Project, ProjectMember + + +@pytest.fixture +def project(db, workspace, create_user): + """Create a test project with the user as a member""" + project = Project.objects.create( + name="Test Project", + identifier="TP", + workspace=workspace, + created_by=create_user, + ) + ProjectMember.objects.create( + project=project, + member=create_user, + role=20, # Admin role + is_active=True, + ) + return project + + +@pytest.fixture +def cycle_data(): + """Sample cycle data for tests""" + return { + "name": "Test Cycle", + "description": "A test cycle for unit tests", + } + + +@pytest.fixture +def draft_cycle_data(): + """Sample draft cycle data (no dates)""" + return { + "name": "Draft Cycle", + "description": "A draft cycle without dates", + } + + +@pytest.fixture +def create_cycle(db, project, create_user): + """Create a test cycle""" + return Cycle.objects.create( + name="Existing Cycle", + description="An existing cycle", + start_date=timezone.now() + timedelta(days=1), + end_date=timezone.now() + timedelta(days=7), + project=project, + workspace=project.workspace, + owned_by=create_user, + ) + + + + +@pytest.mark.contract +class TestCycleListCreateAPIEndpoint: + """Test Cycle List and Create API Endpoint""" + + def get_cycle_url(self, workspace_slug, project_id): + """Helper to get cycle endpoint URL""" + return f"/api/v1/workspaces/{workspace_slug}/projects/{project_id}/cycles/" + + @pytest.mark.django_db + def test_create_cycle_success(self, api_key_client, workspace, project, cycle_data): + """Test successful cycle creation""" + url = self.get_cycle_url(workspace.slug, project.id) + + response = api_key_client.post(url, cycle_data, format="json") + + assert response.status_code == status.HTTP_201_CREATED + + assert Cycle.objects.count() == 1 + + created_cycle = Cycle.objects.first() + assert created_cycle.name == cycle_data["name"] + assert created_cycle.description == cycle_data["description"] + assert created_cycle.project == project + assert created_cycle.owned_by_id is not None + + + @pytest.mark.django_db + def test_create_cycle_invalid_data(self, api_key_client, workspace, project): + """Test cycle creation with invalid data""" + url = self.get_cycle_url(workspace.slug, project.id) + + # Test with empty data + response = api_key_client.post(url, {}, format="json") + assert response.status_code == status.HTTP_400_BAD_REQUEST + + # Test with missing name + response = api_key_client.post(url, {"description": "Test cycle"}, format="json") + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @pytest.mark.django_db + def test_create_cycle_invalid_date_combination(self, api_key_client, workspace, project): + """Test cycle creation with invalid date combination (only start_date)""" + url = self.get_cycle_url(workspace.slug, project.id) + + invalid_data = { + "name": "Invalid Cycle", + "start_date": (timezone.now() + timedelta(days=1)).isoformat(), + # Missing end_date + } + + response = api_key_client.post(url, invalid_data, format="json") + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "Both start date and end date are either required or are to be null" in response.data["error"] + + @pytest.mark.django_db + def test_create_cycle_with_external_id(self, api_key_client, workspace, project): + """Test creating cycle with external ID""" + url = self.get_cycle_url(workspace.slug, project.id) + + cycle_data = { + "name": "External Cycle", + "description": "A cycle with external ID", + "external_id": "ext-123", + "external_source": "github", + } + + response = api_key_client.post(url, cycle_data, format="json") + + assert response.status_code == status.HTTP_201_CREATED + created_cycle = Cycle.objects.first() + assert created_cycle.external_id == "ext-123" + assert created_cycle.external_source == "github" + + @pytest.mark.django_db + def test_create_cycle_duplicate_external_id(self, api_key_client, workspace, project, create_user): + """Test creating cycle with duplicate external ID""" + url = self.get_cycle_url(workspace.slug, project.id) + + # Create first cycle + Cycle.objects.create( + name="First Cycle", + project=project, + workspace=workspace, + external_id="ext-123", + external_source="github", + owned_by=create_user, + ) + + # Try to create second cycle with same external ID + cycle_data = { + "name": "Second Cycle", + "external_id": "ext-123", + "external_source": "github", + "owned_by": create_user.id, + } + + response = api_key_client.post(url, cycle_data, format="json") + + assert response.status_code == status.HTTP_409_CONFLICT + assert "same external id" in response.data["error"] + + @pytest.mark.django_db + def test_list_cycles_success(self, api_key_client, workspace, project, create_cycle, create_user): + """Test successful cycle listing""" + url = self.get_cycle_url(workspace.slug, project.id) + + # Create additional cycles + Cycle.objects.create( + name="Cycle 2", + project=project, + workspace=workspace, + start_date=timezone.now() + timedelta(days=10), + end_date=timezone.now() + timedelta(days=17), + owned_by=create_user, + ) + Cycle.objects.create( + name="Cycle 3", + project=project, + workspace=workspace, + start_date=timezone.now() + timedelta(days=20), + end_date=timezone.now() + timedelta(days=27), + owned_by=create_user, + ) + + response = api_key_client.get(url) + + assert response.status_code == status.HTTP_200_OK + assert "results" in response.data + assert len(response.data["results"]) == 3 # Including create_cycle fixture + + @pytest.mark.django_db + def test_list_cycles_with_view_filter(self, api_key_client, workspace, project, create_user): + """Test cycle listing with different view filters""" + url = self.get_cycle_url(workspace.slug, project.id) + + # Create cycles in different states + now = timezone.now() + + # Current cycle (started but not ended) + Cycle.objects.create( + name="Current Cycle", + project=project, + workspace=workspace, + start_date=now - timedelta(days=1), + end_date=now + timedelta(days=6), + owned_by=create_user, + ) + + # Upcoming cycle + Cycle.objects.create( + name="Upcoming Cycle", + project=project, + workspace=workspace, + start_date=now + timedelta(days=1), + end_date=now + timedelta(days=8), + owned_by=create_user, + ) + + # Completed cycle + Cycle.objects.create( + name="Completed Cycle", + project=project, + workspace=workspace, + start_date=now - timedelta(days=10), + end_date=now - timedelta(days=3), + owned_by=create_user, + ) + + # Draft cycle + Cycle.objects.create( + name="Draft Cycle", + project=project, + workspace=workspace, + owned_by=create_user, + ) + + # Test current cycles + response = api_key_client.get(url, {"cycle_view": "current"}) + assert response.status_code == status.HTTP_200_OK + assert len(response.data) == 1 + assert response.data[0]["name"] == "Current Cycle" + + # Test upcoming cycles + response = api_key_client.get(url, {"cycle_view": "upcoming"}) + assert response.status_code == status.HTTP_200_OK + assert len(response.data["results"]) == 1 + assert response.data["results"][0]["name"] == "Upcoming Cycle" + + # Test completed cycles + response = api_key_client.get(url, {"cycle_view": "completed"}) + assert response.status_code == status.HTTP_200_OK + assert len(response.data["results"]) == 1 + assert response.data["results"][0]["name"] == "Completed Cycle" + + # Test draft cycles + response = api_key_client.get(url, {"cycle_view": "draft"}) + assert response.status_code == status.HTTP_200_OK + assert len(response.data["results"]) == 1 + assert response.data["results"][0]["name"] == "Draft Cycle" + + +@pytest.mark.contract +class TestCycleDetailAPIEndpoint: + """Test Cycle Detail API Endpoint""" + + def get_cycle_detail_url(self, workspace_slug, project_id, cycle_id): + """Helper to get cycle detail endpoint URL""" + return f"/api/v1/workspaces/{workspace_slug}/projects/{project_id}/cycles/{cycle_id}/" + + @pytest.mark.django_db + def test_get_cycle_success(self, api_key_client, workspace, project, create_cycle): + """Test successful cycle retrieval""" + url = self.get_cycle_detail_url(workspace.slug, project.id, create_cycle.id) + + response = api_key_client.get(url) + + assert response.status_code == status.HTTP_200_OK + assert str(response.data["id"]) == str(create_cycle.id) + assert response.data["name"] == create_cycle.name + assert response.data["description"] == create_cycle.description + + @pytest.mark.django_db + def test_get_cycle_not_found(self, api_key_client, workspace, project): + """Test getting non-existent cycle""" + fake_id = uuid4() + url = self.get_cycle_detail_url(workspace.slug, project.id, fake_id) + + response = api_key_client.get(url) + assert response.status_code == status.HTTP_404_NOT_FOUND + + @pytest.mark.django_db + def test_update_cycle_success(self, api_key_client, workspace, project, create_cycle): + """Test successful cycle update""" + url = self.get_cycle_detail_url(workspace.slug, project.id, create_cycle.id) + + update_data = { + "name": f"Updated Cycle {uuid4()}", + "description": "Updated description", + } + + response = api_key_client.patch(url, update_data, format="json") + + assert response.status_code == status.HTTP_200_OK + + create_cycle.refresh_from_db() + assert create_cycle.name == update_data["name"] + assert create_cycle.description == update_data["description"] + + @pytest.mark.django_db + def test_update_cycle_invalid_data(self, api_key_client, workspace, project, create_cycle): + """Test cycle update with invalid data""" + url = self.get_cycle_detail_url(workspace.slug, project.id, create_cycle.id) + + update_data = {"name": ""} + response = api_key_client.patch(url, update_data, format="json") + + # This might be 400 if name is required, or 200 if empty names are allowed + assert response.status_code in [status.HTTP_400_BAD_REQUEST, status.HTTP_200_OK] + + @pytest.mark.django_db + def test_update_cycle_with_external_id_conflict(self, api_key_client, workspace, project, create_cycle, create_user ): + """Test cycle update with conflicting external ID""" + url = self.get_cycle_detail_url(workspace.slug, project.id, create_cycle.id) + + # Create another cycle with external ID + Cycle.objects.create( + name="Another Cycle", + project=project, + workspace=workspace, + external_id="ext-456", + external_source="github", + owned_by=create_user, + ) + + # Try to update cycle with same external ID + update_data = { + "external_id": "ext-456", + "external_source": "github", + } + + response = api_key_client.patch(url, update_data, format="json") + + assert response.status_code == status.HTTP_409_CONFLICT + assert "same external id" in response.data["error"] + + @pytest.mark.django_db + def test_delete_cycle_success(self, api_key_client, workspace, project, create_cycle): + """Test successful cycle deletion""" + url = self.get_cycle_detail_url(workspace.slug, project.id, create_cycle.id) + + response = api_key_client.delete(url) + + assert response.status_code == status.HTTP_204_NO_CONTENT + assert not Cycle.objects.filter(id=create_cycle.id).exists() + + @pytest.mark.django_db + def test_cycle_metrics_annotation(self, api_key_client, workspace, project, create_cycle): + """Test that cycle includes issue metrics annotations""" + url = self.get_cycle_detail_url(workspace.slug, project.id, create_cycle.id) + + response = api_key_client.get(url) + + assert response.status_code == status.HTTP_200_OK + + # Check that metrics are included in response + cycle_data = response.data + assert "total_issues" in cycle_data + assert "completed_issues" in cycle_data + assert "cancelled_issues" in cycle_data + assert "started_issues" in cycle_data + assert "unstarted_issues" in cycle_data + assert "backlog_issues" in cycle_data + + # All should be 0 for a new cycle + assert cycle_data["total_issues"] == 0 + assert cycle_data["completed_issues"] == 0 + assert cycle_data["cancelled_issues"] == 0 + assert cycle_data["started_issues"] == 0 + assert cycle_data["unstarted_issues"] == 0 + assert cycle_data["backlog_issues"] == 0 \ No newline at end of file