From 88b751dab1500d0ce3389d396344bb2e7936a190 Mon Sep 17 00:00:00 2001 From: Jose Antonio Martinez <257598434+jamartineztelecoengineer84-dotcom@users.noreply.github.com> Date: Mon, 27 Apr 2026 19:55:18 +0000 Subject: [PATCH 1/6] test(api): add regression tests for create-project endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cover three scenarios: - project_lead set to the creator's own user_id - project_lead set to a different workspace member - project_lead omitted (baseline) The first two currently fail on preview because of a UUID coercion bug in ProjectMember.objects.create — see follow-up commit. --- .../plane/tests/contract/api/test_projects.py | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 apps/api/plane/tests/contract/api/test_projects.py diff --git a/apps/api/plane/tests/contract/api/test_projects.py b/apps/api/plane/tests/contract/api/test_projects.py new file mode 100644 index 00000000000..1b95b45d2d8 --- /dev/null +++ b/apps/api/plane/tests/contract/api/test_projects.py @@ -0,0 +1,105 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +import pytest +from rest_framework import status + +from plane.db.models import Project, ProjectMember, State, User, WorkspaceMember + + +@pytest.fixture +def other_workspace_member(db, workspace): + """Create another user that is a member of the workspace, distinct from the creator.""" + from uuid import uuid4 + + unique_id = uuid4().hex[:8] + other = User.objects.create( + email=f"other-{unique_id}@plane.so", + username=f"other_user_{unique_id}", + first_name="Other", + last_name="User", + ) + other.set_password("test-password") + other.save() + WorkspaceMember.objects.create(workspace=workspace, member=other, role=20) + return other + + +@pytest.mark.contract +class TestProjectListCreateAPIEndpoint: + """Contract tests for POST /api/v1/workspaces/{slug}/projects/.""" + + def get_url(self, workspace_slug): + return f"/api/v1/workspaces/{workspace_slug}/projects/" + + @pytest.mark.django_db + def test_create_project_with_lead_as_creator(self, api_key_client, workspace, create_user): + """Regression for the ghost-create bug. + + When project_lead points to the creator's own user_id, the endpoint + must return 201 and create a fully-populated project (single + ProjectMember as admin, default workflow states). + + Before the fix, the endpoint returned 400 "Please provide valid detail" + but had already persisted the Project row without states or members, + leaving an unusable orphan. + """ + url = self.get_url(workspace.slug) + payload = { + "name": "Self Lead Project", + "identifier": "SL", + "project_lead": str(create_user.id), + } + + response = api_key_client.post(url, payload, format="json") + + assert response.status_code == status.HTTP_201_CREATED, f"Got {response.status_code}: {response.data!r}" + assert Project.objects.count() == 1 + + project = Project.objects.first() + # Creator is registered as admin (single membership; lead == creator + # should not produce a duplicate row). + assert ProjectMember.objects.filter(project=project, member=create_user, role=20).count() == 1 + # Default workflow states must be created. + assert State.objects.filter(project=project).count() == 5 + + @pytest.mark.django_db + def test_create_project_with_lead_as_other_user( + self, api_key_client, workspace, create_user, other_workspace_member + ): + """When project_lead is a different workspace member, both creator + and lead become admins of the project.""" + url = self.get_url(workspace.slug) + payload = { + "name": "Other Lead Project", + "identifier": "OL", + "project_lead": str(other_workspace_member.id), + } + + response = api_key_client.post(url, payload, format="json") + + assert response.status_code == status.HTTP_201_CREATED, f"Got {response.status_code}: {response.data!r}" + project = Project.objects.first() + + # Both creator and other_workspace_member are admins. + assert ProjectMember.objects.filter(project=project, member=create_user, role=20).exists() + assert ProjectMember.objects.filter(project=project, member=other_workspace_member, role=20).exists() + assert State.objects.filter(project=project).count() == 5 + + @pytest.mark.django_db + def test_create_project_without_lead(self, api_key_client, workspace, create_user): + """Baseline regression: omitting project_lead must succeed and the + creator becomes the sole admin.""" + url = self.get_url(workspace.slug) + payload = { + "name": "Basic Project", + "identifier": "BP", + } + + response = api_key_client.post(url, payload, format="json") + + assert response.status_code == status.HTTP_201_CREATED, f"Got {response.status_code}: {response.data!r}" + project = Project.objects.first() + assert ProjectMember.objects.filter(project=project, member=create_user, role=20).count() == 1 + assert State.objects.filter(project=project).count() == 5 From 4d606fec6648f454aa546cf4efeaa4a04ddf4ae0 Mon Sep 17 00:00:00 2001 From: Jose Antonio Martinez <257598434+jamartineztelecoengineer84-dotcom@users.noreply.github.com> Date: Mon, 27 Apr 2026 20:08:59 +0000 Subject: [PATCH 2/6] fix(api): pass project_lead_id (not User instance) when creating ProjectMember MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The create-project endpoint built a ProjectMember row with member_id=serializer.instance.project_lead, which resolves to a User instance via Django's related descriptor instead of a UUID. Django's UUIDField coercion then fails with AttributeError: 'User' object has no attribute 'replace', which the generic exception handler converts to a 400 "Please provide valid detail" — but only after the Project row was already persisted, leaving an orphaned project without default states. Fix: - Use project_lead_id (FK ID, no descriptor lookup) on both the guard comparison and the ProjectMember creation. - Wrap the post-save flow in transaction.atomic() so any future exception triggers a clean rollback. - Defer model_activity.delay() with transaction.on_commit() so the activity log only fires after a successful commit. - Capture the exception with log_exception() in the generic catch so future regressions surface in api logs. Note: a related data integrity issue exists where ProjectCreateSerializer doesn't create a ProjectIdentifier row (unlike its frontend counterpart). Out of scope here, will follow up in a separate PR. --- apps/api/plane/api/views/project.py | 98 +++++++++++++++++------------ 1 file changed, 59 insertions(+), 39 deletions(-) diff --git a/apps/api/plane/api/views/project.py b/apps/api/plane/api/views/project.py index 5ab0fd1c1fa..9c04dda98ee 100644 --- a/apps/api/plane/api/views/project.py +++ b/apps/api/plane/api/views/project.py @@ -6,7 +6,7 @@ import json # Django imports -from django.db import IntegrityError +from django.db import IntegrityError, transaction from django.db.models import Exists, F, Func, OuterRef, Prefetch, Q, Subquery, Count from django.db.models.functions import Coalesce from django.utils import timezone @@ -38,6 +38,7 @@ ProjectPage, ) from plane.bgtasks.webhook_task import model_activity, webhook_activity +from plane.utils.exception_logger import log_exception from .base import BaseAPIView from plane.utils.host import base_host from plane.api.serializers import ( @@ -223,48 +224,59 @@ def post(self, request, slug): serializer = ProjectCreateSerializer(data={**request.data}, context={"workspace_id": workspace.id}) if serializer.is_valid(): - serializer.save() - - # Add the user as Administrator to the project - _ = ProjectMember.objects.create(project_id=serializer.instance.id, member=request.user, role=20) + with transaction.atomic(): + serializer.save() + + # Add the creator as Administrator of the project. + _ = ProjectMember.objects.create(project_id=serializer.instance.id, member=request.user, role=20) + + # If a different project_lead was provided, add them as + # Administrator too. Use project_lead_id (the FK column) + # rather than project_lead (the related descriptor, which + # would resolve to a User instance and break UUID coercion + # downstream in ProjectMember.objects.create). + if ( + serializer.instance.project_lead_id is not None + and serializer.instance.project_lead_id != request.user.id + ): + ProjectMember.objects.create( + project_id=serializer.instance.id, + member_id=serializer.instance.project_lead_id, + role=20, + ) - if serializer.instance.project_lead is not None and str(serializer.instance.project_lead) != str( - request.user.id - ): - ProjectMember.objects.create( - project_id=serializer.instance.id, - member_id=serializer.instance.project_lead, - role=20, + State.objects.bulk_create( + [ + State( + name=state["name"], + color=state["color"], + project=serializer.instance, + sequence=state["sequence"], + workspace=serializer.instance.workspace, + group=state["group"], + default=state.get("default", False), + created_by=request.user, + ) + for state in DEFAULT_STATES + ] ) - State.objects.bulk_create( - [ - State( - name=state["name"], - color=state["color"], - project=serializer.instance, - sequence=state["sequence"], - workspace=serializer.instance.workspace, - group=state["group"], - default=state.get("default", False), - created_by=request.user, + project = self.get_queryset().filter(pk=serializer.instance.id).first() + + # Defer the activity-log task until the surrounding + # transaction commits, so it never fires on a rolled-back + # creation. + transaction.on_commit( + lambda: model_activity.delay( + model_name="project", + model_id=str(project.id), + requested_data=request.data, + current_instance=None, + actor_id=request.user.id, + slug=slug, + origin=base_host(request=request, is_app=True), ) - for state in DEFAULT_STATES - ] - ) - - project = self.get_queryset().filter(pk=serializer.instance.id).first() - - # Model activity - model_activity.delay( - model_name="project", - model_id=str(project.id), - requested_data=request.data, - current_instance=None, - actor_id=request.user.id, - slug=slug, - origin=base_host(request=request, is_app=True), - ) + ) serializer = ProjectSerializer(project) return Response(serializer.data, status=status.HTTP_201_CREATED) @@ -282,6 +294,14 @@ def post(self, request, slug): {"identifier": "The project identifier is already taken"}, status=status.HTTP_409_CONFLICT, ) + except Exception as e: + # Surface unexpected failures in the api logs so future regressions + # are debuggable. Keep the public response generic. + log_exception(e) + return Response( + {"error": "Please provide valid detail"}, + status=status.HTTP_400_BAD_REQUEST, + ) class ProjectDetailAPIEndpoint(BaseAPIView): From 6a85b5f751a40a62281402416f0a983e15e56acd Mon Sep 17 00:00:00 2001 From: Jose Antonio Martinez <257598434+jamartineztelecoengineer84-dotcom@users.noreply.github.com> Date: Wed, 29 Apr 2026 09:20:23 +0000 Subject: [PATCH 3/6] fix(api): return 500 on unexpected errors and harden project create Address review feedback from @sriramveeraghanta on PR #8966: - The catch-all `except Exception` now returns 500 instead of 400. Reusing the generic 400 response on a server-side crash was the anti-pattern that hid the original ghost-create bug for nine months; a 500 lets clients distinguish between "bad input" and "server fault". - The `IntegrityError` branch no longer falls through silently when the message is unrecognised. It re-raises so the catch-all `except` logs the exception and returns a 500. - `transaction.on_commit()` now schedules `model_activity.delay` via `functools.partial` instead of a lambda, avoiding late-binding closure semantics. - `ProjectCreateSerializer.validate()` now rejects `project_lead` values that are not active workspace members, surfacing the error under the `project_lead` field key (rather than as `non_field_errors`) so API clients can react programmatically. --- apps/api/plane/api/serializers/project.py | 19 +++++++++++++------ apps/api/plane/api/views/project.py | 22 ++++++++++++++++------ 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/apps/api/plane/api/serializers/project.py b/apps/api/plane/api/serializers/project.py index 644b5ba1076..aa98dd439fe 100644 --- a/apps/api/plane/api/serializers/project.py +++ b/apps/api/plane/api/serializers/project.py @@ -114,13 +114,20 @@ def validate(self, data): if project_identifier is not None and re.match(Project.FORBIDDEN_IDENTIFIER_CHARS_PATTERN, project_identifier): raise serializers.ValidationError("Project identifier cannot contain special characters.") - if data.get("project_lead", None) is not None: - # Check if the project lead is a member of the workspace - if not WorkspaceMember.objects.filter( + project_lead = data.get("project_lead") + if ( + project_lead + and not WorkspaceMember.objects.filter( workspace_id=self.context["workspace_id"], - member_id=data.get("project_lead"), - ).exists(): - raise serializers.ValidationError("Project lead should be a user in the workspace") + member=project_lead, + is_active=True, + ).exists() + ): + # Field-shaped error so DRF surfaces it under the specific key + # rather than as non_field_errors. Also requires the membership + # to be active so that revoked / removed members can't slip + # through and trigger the FK error downstream. + raise serializers.ValidationError({"project_lead": "The provided user is not a member of this workspace."}) if data.get("default_assignee", None) is not None: # Check if the default assignee is a member of the workspace diff --git a/apps/api/plane/api/views/project.py b/apps/api/plane/api/views/project.py index 9c04dda98ee..6676d2c5536 100644 --- a/apps/api/plane/api/views/project.py +++ b/apps/api/plane/api/views/project.py @@ -4,6 +4,7 @@ # Python imports import json +from functools import partial # Django imports from django.db import IntegrityError, transaction @@ -265,9 +266,12 @@ def post(self, request, slug): # Defer the activity-log task until the surrounding # transaction commits, so it never fires on a rolled-back - # creation. + # creation. Use functools.partial so the bound arguments + # are captured at scheduling time rather than via a + # late-binding closure. transaction.on_commit( - lambda: model_activity.delay( + partial( + model_activity.delay, model_name="project", model_id=str(project.id), requested_data=request.data, @@ -287,6 +291,10 @@ def post(self, request, slug): {"name": "The project name is already taken"}, status=status.HTTP_409_CONFLICT, ) + # Any other IntegrityError is unexpected — let the catch-all + # `Exception` branch below log it and return 500 instead of + # falling through to an implicit 200 with no body. + raise except Workspace.DoesNotExist: return Response({"error": "Workspace does not exist"}, status=status.HTTP_404_NOT_FOUND) except ValidationError: @@ -295,12 +303,14 @@ def post(self, request, slug): status=status.HTTP_409_CONFLICT, ) except Exception as e: - # Surface unexpected failures in the api logs so future regressions - # are debuggable. Keep the public response generic. + # Unexpected server-side failure: log the traceback and return a + # generic 500 so the client can distinguish it from a 4xx caused + # by bad input. Returning 400 here was the anti-pattern that + # masked the original ghost-create bug. log_exception(e) return Response( - {"error": "Please provide valid detail"}, - status=status.HTTP_400_BAD_REQUEST, + {"error": "An unexpected error occurred"}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR, ) From acb91466f41d4d0dc0df0b6293111140558026f7 Mon Sep 17 00:00:00 2001 From: Jose Antonio Martinez <257598434+jamartineztelecoengineer84-dotcom@users.noreply.github.com> Date: Wed, 29 Apr 2026 09:20:24 +0000 Subject: [PATCH 4/6] test(api): harden assertions and cover rollback / workspace-membership Address review feedback from @sriramveeraghanta on PR #8966: - The three existing tests now look up the created project via `Project.objects.get(id=response.data["id"])` instead of `.first()`. The assertion now fails for the right reason if the wrong project is returned by the endpoint. - New `test_create_project_with_lead_not_in_workspace_returns_400` guards the workspace-membership validation added to `ProjectCreateSerializer.validate()`. Expects a 400 with a field-shaped error and zero rows persisted. - New `test_model_activity_not_called_on_rollback` locks in the `transaction.on_commit()` semantics: when an exception is raised inside the atomic block (forced via mocking `State.objects.bulk_create`), the response is 500, no Project / ProjectMember / State rows are persisted, and the deferred `model_activity.delay` task is never dispatched. This prevents a future refactor from silently regressing the rollback contract. --- .../plane/tests/contract/api/test_projects.py | 91 +++++++++++++++++-- 1 file changed, 85 insertions(+), 6 deletions(-) diff --git a/apps/api/plane/tests/contract/api/test_projects.py b/apps/api/plane/tests/contract/api/test_projects.py index 1b95b45d2d8..e8f20869375 100644 --- a/apps/api/plane/tests/contract/api/test_projects.py +++ b/apps/api/plane/tests/contract/api/test_projects.py @@ -2,6 +2,9 @@ # SPDX-License-Identifier: AGPL-3.0-only # See the LICENSE file for details. +from unittest import mock +from uuid import uuid4 + import pytest from rest_framework import status @@ -11,8 +14,6 @@ @pytest.fixture def other_workspace_member(db, workspace): """Create another user that is a member of the workspace, distinct from the creator.""" - from uuid import uuid4 - unique_id = uuid4().hex[:8] other = User.objects.create( email=f"other-{unique_id}@plane.so", @@ -26,6 +27,21 @@ def other_workspace_member(db, workspace): return other +@pytest.fixture +def outsider_user(db): + """Create a user that is NOT a member of any workspace under test.""" + unique_id = uuid4().hex[:8] + outsider = User.objects.create( + email=f"outsider-{unique_id}@plane.so", + username=f"outsider_{unique_id}", + first_name="Out", + last_name="Sider", + ) + outsider.set_password("test-password") + outsider.save() + return outsider + + @pytest.mark.contract class TestProjectListCreateAPIEndpoint: """Contract tests for POST /api/v1/workspaces/{slug}/projects/.""" @@ -55,9 +71,10 @@ def test_create_project_with_lead_as_creator(self, api_key_client, workspace, cr response = api_key_client.post(url, payload, format="json") assert response.status_code == status.HTTP_201_CREATED, f"Got {response.status_code}: {response.data!r}" - assert Project.objects.count() == 1 - project = Project.objects.first() + # Look up the project we just created instead of relying on + # ordering-sensitive Project.objects.first(). + project = Project.objects.get(id=response.data["id"]) # Creator is registered as admin (single membership; lead == creator # should not produce a duplicate row). assert ProjectMember.objects.filter(project=project, member=create_user, role=20).count() == 1 @@ -80,7 +97,7 @@ def test_create_project_with_lead_as_other_user( response = api_key_client.post(url, payload, format="json") assert response.status_code == status.HTTP_201_CREATED, f"Got {response.status_code}: {response.data!r}" - project = Project.objects.first() + project = Project.objects.get(id=response.data["id"]) # Both creator and other_workspace_member are admins. assert ProjectMember.objects.filter(project=project, member=create_user, role=20).exists() @@ -100,6 +117,68 @@ def test_create_project_without_lead(self, api_key_client, workspace, create_use response = api_key_client.post(url, payload, format="json") assert response.status_code == status.HTTP_201_CREATED, f"Got {response.status_code}: {response.data!r}" - project = Project.objects.first() + project = Project.objects.get(id=response.data["id"]) assert ProjectMember.objects.filter(project=project, member=create_user, role=20).count() == 1 assert State.objects.filter(project=project).count() == 5 + + @pytest.mark.django_db + def test_create_project_with_lead_not_in_workspace_returns_400(self, api_key_client, workspace, outsider_user): + """When project_lead refers to a user that is NOT a member of the + target workspace, the endpoint must reject the request with a 400 + carrying a field-shaped error and must not persist the Project.""" + url = self.get_url(workspace.slug) + payload = { + "name": "Outsider Lead Project", + "identifier": "OUT", + "project_lead": str(outsider_user.id), + } + + response = api_key_client.post(url, payload, format="json") + + assert response.status_code == status.HTTP_400_BAD_REQUEST, f"Got {response.status_code}: {response.data!r}" + assert "project_lead" in response.data, ( + f"Expected field-shaped error under 'project_lead', got {response.data!r}" + ) + # No project should have been persisted. + assert Project.objects.count() == 0 + + @pytest.mark.django_db + def test_model_activity_not_called_on_rollback(self, api_key_client, workspace, create_user): + """If anything inside the transaction.atomic() block raises, the + whole creation must roll back (no Project, no ProjectMember, no + State) and the deferred model_activity.delay() task must not fire, + because it is registered with transaction.on_commit(). + + Force the failure inside State.objects.bulk_create — past the point + where the original ghost-create bug would have committed a partial + Project — and verify the response is 500 with no side effects. + """ + url = self.get_url(workspace.slug) + payload = { + "name": "Rollback Probe", + "identifier": "RB", + "project_lead": str(create_user.id), + } + + forced_error = RuntimeError("forced failure for rollback test") + + with ( + mock.patch( + "plane.api.views.project.State.objects.bulk_create", + side_effect=forced_error, + ), + mock.patch("plane.api.views.project.model_activity") as mocked_activity, + ): + response = api_key_client.post(url, payload, format="json") + + assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR, ( + f"Got {response.status_code}: {response.data!r}" + ) + # Transaction must have rolled back: no Project, no ProjectMember, + # no State persisted. + assert Project.objects.count() == 0 + assert ProjectMember.objects.count() == 0 + assert State.objects.count() == 0 + # And the deferred Celery task must not have been dispatched — + # transaction.on_commit() callbacks only fire on a successful commit. + mocked_activity.delay.assert_not_called() From 03e052e242b4bfd36538045f192818fab908b2d8 Mon Sep 17 00:00:00 2001 From: Jose Antonio Martinez <257598434+jamartineztelecoengineer84-dotcom@users.noreply.github.com> Date: Wed, 29 Apr 2026 10:18:00 +0000 Subject: [PATCH 5/6] fix(api): mark on_commit dispatch as robust against broker failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address coderabbit re-review feedback on PR #8966. Without robust=True, an exception raised by model_activity.delay (e.g., a Celery broker outage) propagates out of the on_commit callback and is caught by the outer `except Exception` handler, which returns a 500 despite the project, ProjectMember rows and default States having already been committed. The client sees a 500 and assumes the create failed — the same class of mismatch between actual state and reported status that the original bug exhibited, just at the post-commit phase. Set robust=True so Django logs the dispatch failure internally via the standard transaction logger and the response stays 201, reflecting the persisted state. Switch from `functools.partial` to a nested function (`_dispatch_model_activity`) for the on_commit callable. Django's robust on_commit logging path reads `func.__qualname__` to format the error message; `partial` objects lack that dunder by default, and the `functools.update_wrapper` workaround turns out to be brittle when the wrapped callable is replaced by a Mock (which the new regression test relies on). A nested function exposes `__qualname__` natively, and the locals it closes over are bound at definition time and never rebound before the callback fires, so the late-binding-closure motivation for `partial` over `lambda` does not apply here. A new test, test_response_still_201_when_broker_dispatch_fails, mirrors test_model_activity_not_called_on_rollback to lock in the post-commit branch. It uses `@pytest.mark.django_db(transaction=True)` so the surrounding test transaction is actually committed and the `on_commit` callback fires (the default wrapper suppresses it via rollback). --- apps/api/plane/api/views/project.py | 25 ++++++++++----- .../plane/tests/contract/api/test_projects.py | 32 +++++++++++++++++++ 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/apps/api/plane/api/views/project.py b/apps/api/plane/api/views/project.py index 6676d2c5536..ae6ccdb98f1 100644 --- a/apps/api/plane/api/views/project.py +++ b/apps/api/plane/api/views/project.py @@ -4,7 +4,6 @@ # Python imports import json -from functools import partial # Django imports from django.db import IntegrityError, transaction @@ -266,12 +265,21 @@ def post(self, request, slug): # Defer the activity-log task until the surrounding # transaction commits, so it never fires on a rolled-back - # creation. Use functools.partial so the bound arguments - # are captured at scheduling time rather than via a - # late-binding closure. - transaction.on_commit( - partial( - model_activity.delay, + # creation. + # robust=True so broker / dispatch failures are logged + # internally by Django and don't surface as 500 after a + # successful commit (the inverse of the rollback path + # covered by test_model_activity_not_called_on_rollback). + # A nested function (rather than functools.partial) is + # used here because Django's robust on_commit logging + # path reads ``func.__qualname__`` to format the error + # message; ``partial`` objects don't have that dunder + # by default and the workaround is brittle when the + # wrapped callable is a mock. The closure captures + # the locals at construction time and they are never + # rebound, so late-binding is not a hazard here. + def _dispatch_model_activity(): + model_activity.delay( model_name="project", model_id=str(project.id), requested_data=request.data, @@ -280,7 +288,8 @@ def post(self, request, slug): slug=slug, origin=base_host(request=request, is_app=True), ) - ) + + transaction.on_commit(_dispatch_model_activity, robust=True) serializer = ProjectSerializer(project) return Response(serializer.data, status=status.HTTP_201_CREATED) diff --git a/apps/api/plane/tests/contract/api/test_projects.py b/apps/api/plane/tests/contract/api/test_projects.py index e8f20869375..329097e60cd 100644 --- a/apps/api/plane/tests/contract/api/test_projects.py +++ b/apps/api/plane/tests/contract/api/test_projects.py @@ -182,3 +182,35 @@ def test_model_activity_not_called_on_rollback(self, api_key_client, workspace, # And the deferred Celery task must not have been dispatched — # transaction.on_commit() callbacks only fire on a successful commit. mocked_activity.delay.assert_not_called() + + @pytest.mark.django_db(transaction=True) + def test_response_still_201_when_broker_dispatch_fails(self, api_key_client, workspace, create_user): + """If model_activity.delay raises *after* the atomic block has + committed (e.g., the Celery broker is down), the project, member + rows and states are already persisted — the response must remain + 201 and the failure must be absorbed by Django's robust=True + on_commit handling, not surface as a 500. + + Uses ``transaction=True`` so the surrounding test transaction is + actually committed and the ``on_commit`` callback fires (the + default ``django_db`` wrapper would suppress it via rollback).""" + url = self.get_url(workspace.slug) + payload = { + "name": "Broker Down", + "identifier": "BD", + "project_lead": str(create_user.id), + } + + with mock.patch("plane.api.views.project.model_activity") as mocked_activity: + mocked_activity.delay.side_effect = RuntimeError("broker unavailable") + response = api_key_client.post(url, payload, format="json") + + assert response.status_code == status.HTTP_201_CREATED, f"Got {response.status_code}: {response.data!r}" + # Project and its scaffolding are persisted (commit happened + # before the on_commit callback fired). + project = Project.objects.get(id=response.data["id"]) + assert ProjectMember.objects.filter(project=project).count() == 1 + assert State.objects.filter(project=project).count() == 5 + # The dispatch was attempted but its failure was swallowed by + # transaction.on_commit(robust=True). + mocked_activity.delay.assert_called_once() From 821eb150e86b86083f5f248594c153ad365b77bc Mon Sep 17 00:00:00 2001 From: Jose Antonio Martinez <257598434+jamartineztelecoengineer84-dotcom@users.noreply.github.com> Date: Wed, 29 Apr 2026 10:35:41 +0000 Subject: [PATCH 6/6] fix(api): handle unrecognised IntegrityError consistently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address coderabbit re-review feedback on PR #8966. The previous fix used `raise` inside the IntegrityError handler with the intent of "letting the catch-all `except Exception` below log it and return 500". Coderabbit correctly flagged that `raise` exits the try/except entirely — sibling except clauses don't fire — so unrecognised integrity errors actually skipped `log_exception` and the consistent 500 JSON shape, contradicting the stated intent. Replicate the catch-all behaviour inline: log the exception via `log_exception(e)` and return the same generic 500 response with `{"error": "An unexpected error occurred"}`. The client now gets a uniform error shape regardless of which `except` branch handled it. --- apps/api/plane/api/views/project.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/apps/api/plane/api/views/project.py b/apps/api/plane/api/views/project.py index ae6ccdb98f1..bc478d1bb10 100644 --- a/apps/api/plane/api/views/project.py +++ b/apps/api/plane/api/views/project.py @@ -300,10 +300,17 @@ def _dispatch_model_activity(): {"name": "The project name is already taken"}, status=status.HTTP_409_CONFLICT, ) - # Any other IntegrityError is unexpected — let the catch-all - # `Exception` branch below log it and return 500 instead of - # falling through to an implicit 200 with no body. - raise + # Any other IntegrityError is unexpected: log it the same way + # the catch-all `except Exception` below would and return the + # same generic 500 so the client gets a uniform error shape. + # `raise` here would not fall through to a sibling except + # clause — it would exit the try/except entirely and bypass + # both the logging and the JSON response. + log_exception(e) + return Response( + {"error": "An unexpected error occurred"}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR, + ) except Workspace.DoesNotExist: return Response({"error": "Workspace does not exist"}, status=status.HTTP_404_NOT_FOUND) except ValidationError: