From afb00338953beaa54551aeb87b74e6b82d23a7a9 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Wed, 19 Aug 2026 09:59:48 -0400 Subject: [PATCH 1/3] test: reset the shared api-key throttle bucket between tests Every external-API test authenticates with the same hardcoded token, and ApiKeyRateThrottle keys its request history on that token in the Django cache. The cache is a real Redis and is not rolled back with the database, so the history accumulates across the whole session until the suite crosses API_KEY_RATE_LIMIT (60/minute) and later tests fail with 429s unrelated to what they assert. That makes the suite order- and size-dependent: adding tests anywhere can break unrelated ones further down, which is exactly what happens when the API notification coverage in the next commit is added. Clear the bucket around each test so the limit is per-test, which is what these tests already assume. --- apps/api/plane/tests/conftest.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/apps/api/plane/tests/conftest.py b/apps/api/plane/tests/conftest.py index 870779c42d6..7fa564373d3 100644 --- a/apps/api/plane/tests/conftest.py +++ b/apps/api/plane/tests/conftest.py @@ -9,6 +9,33 @@ from plane.db.models import User, Workspace, WorkspaceMember from plane.db.models.api import APIToken +# Every external-API test authenticates with this one token, so they all share a +# single ``ApiKeyRateThrottle`` bucket keyed on it. +TEST_API_TOKEN = "test-api-token-12345" + + +@pytest.fixture(autouse=True) +def reset_api_key_throttle(): + """Give each test the full external-API rate limit. + + ``ApiKeyRateThrottle`` keys its bucket on the API token and stores the + request history in the Django cache, which is a real Redis in CI and is not + rolled back with the database. Because every test reuses ``TEST_API_TOKEN``, + the history accumulates across the session until the suite crosses + ``API_KEY_RATE_LIMIT`` (60/minute) and later tests start failing with 429s + that have nothing to do with what they assert. + + That makes the suite order- and size-dependent: adding tests anywhere can + break unrelated ones further down. Clearing the bucket around each test + keeps the limit per-test, which is what these tests assume. + """ + from django.core.cache import cache + + key = f"api_key:{TEST_API_TOKEN}" + cache.delete(key) + yield + cache.delete(key) + @pytest.fixture(scope="session") def django_db_setup(django_db_setup): # noqa: F811 @@ -52,7 +79,7 @@ def api_token(db, create_user): token = APIToken.objects.create( user=create_user, label="Test API Token", - token="test-api-token-12345", + token=TEST_API_TOKEN, ) return token From fb507147c15548b749c223b33788bf919ae66011 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Wed, 19 Aug 2026 09:59:48 -0400 Subject: [PATCH 2/3] fix(api): notify subscribers on comment, link and work item delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #9307 fixed notifications for work items created or updated through the external REST API, but the same omission was left on every other write path in plane/api/views/issue.py: comment create/update/delete, link create/update/delete and work item delete dispatched issue_activity without notification=True. The flag is what gates notifications.delay(...) in issue_activities_task, so those paths recorded activity and fired webhooks while subscribers were never notified and no email was sent. Commenting through the API was therefore silent, while the identical action in the web app notified normally — plane/app/views/issue passes notification=True on all of these paths. Pass notification=True and origin=base_host(...) on the seven remaining sites so the external API matches the web app. Notification fan-out already excludes the actor and is limited to active project members and issue subscribers, and the deliberate opt-outs upstream (bulk update endpoints) are left untouched. Adds contract tests for all seven paths, matching the shape of the ones added with #9307. --- apps/api/plane/api/views/issue.py | 14 ++ .../api/test_comment_link_notifications.py | 204 ++++++++++++++++++ 2 files changed, 218 insertions(+) create mode 100644 apps/api/plane/tests/contract/api/test_comment_link_notifications.py diff --git a/apps/api/plane/api/views/issue.py b/apps/api/plane/api/views/issue.py index da9edc66d66..0155327f5a2 100644 --- a/apps/api/plane/api/views/issue.py +++ b/apps/api/plane/api/views/issue.py @@ -871,6 +871,8 @@ def delete(self, request, slug, project_id, pk): 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) @@ -1208,6 +1210,8 @@ def post(self, request, slug, project_id, issue_id): actor_id=str(link.created_by_id), current_instance=None, epoch=int(timezone.now().timestamp()), + notification=True, + origin=base_host(request=request, is_app=True), ) serializer = IssueLinkSerializer(link) return Response(serializer.data, status=status.HTTP_201_CREATED) @@ -1319,6 +1323,8 @@ def patch(self, request, slug, project_id, issue_id, pk): project_id=str(project_id), current_instance=current_instance, epoch=int(timezone.now().timestamp()), + notification=True, + origin=base_host(request=request, is_app=True), ) serializer = IssueLinkSerializer(issue_link) return Response(serializer.data, status=status.HTTP_200_OK) @@ -1352,6 +1358,8 @@ def delete(self, request, slug, project_id, issue_id, pk): project_id=str(project_id), current_instance=current_instance, epoch=int(timezone.now().timestamp()), + notification=True, + origin=base_host(request=request, is_app=True), ) issue_link.delete() return Response(status=status.HTTP_204_NO_CONTENT) @@ -1495,6 +1503,8 @@ def post(self, request, slug, project_id, issue_id): project_id=str(self.kwargs.get("project_id")), current_instance=None, epoch=int(timezone.now().timestamp()), + notification=True, + origin=base_host(request=request, is_app=True), ) # Send the model activity @@ -1635,6 +1645,8 @@ def patch(self, request, slug, project_id, issue_id, pk): project_id=str(project_id), current_instance=current_instance, epoch=int(timezone.now().timestamp()), + notification=True, + origin=base_host(request=request, is_app=True), ) # Send the model activity model_activity.delay( @@ -1681,6 +1693,8 @@ def delete(self, request, slug, project_id, issue_id, pk): 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/tests/contract/api/test_comment_link_notifications.py b/apps/api/plane/tests/contract/api/test_comment_link_notifications.py new file mode 100644 index 00000000000..009bcb8251d --- /dev/null +++ b/apps/api/plane/tests/contract/api/test_comment_link_notifications.py @@ -0,0 +1,204 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +"""Notification parity for the remaining external REST API write paths. + +makeplane/plane#9307 fixed this for work item create/update, but the same +omission was left on comments, links, and work item deletion: those endpoints +dispatched ``issue_activity`` without ``notification=True``, so the activity was +recorded and webhooks fired while subscribers were never notified. The web app +(``plane.app.views.issue``) passes the flag on every one of these paths, so the +external API was silently the odd one out. + +These assert the dispatch contract rather than the delivered notification. +``notification=True`` is what gates ``notifications.delay(...)`` in +``issue_activities_task``; the fan-out itself is already covered downstream, and +mocking at the dispatch boundary keeps these tests from depending on Celery. +""" + +from unittest.mock import patch + +import pytest +from rest_framework import status + +from plane.db.models import Issue, IssueComment, IssueLink, Project, ProjectMember, State + + +@pytest.fixture +def project(db, workspace, create_user): + """A project with the user as admin and a default state.""" + 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, is_active=True) + State.objects.create( + name="Backlog", + color="#000000", + group="backlog", + default=True, + project=project, + workspace=workspace, + created_by=create_user, + ) + return project + + +@pytest.fixture +def create_issue(db, project, workspace, create_user): + return Issue.objects.create( + name="Existing Issue", + project=project, + workspace=workspace, + created_by=create_user, + ) + + +@pytest.fixture +def create_comment(db, project, workspace, create_issue, create_user): + return IssueComment.objects.create( + comment_html="

Existing comment

", + issue=create_issue, + project=project, + workspace=workspace, + actor=create_user, + created_by=create_user, + ) + + +@pytest.fixture +def create_link(db, project, workspace, create_issue, create_user): + return IssueLink.objects.create( + url="https://example.com/original", + issue=create_issue, + project=project, + workspace=workspace, + created_by=create_user, + ) + + +def _dispatched(mock): + """The single ``issue_activity.delay`` kwargs dict for this request.""" + mock.delay.assert_called_once() + return mock.delay.call_args.kwargs + + +@pytest.mark.contract +class TestCommentNotificationContract: + """Comment writes through ``/api/v1/...`` must notify, as the web app does.""" + + def url(self, workspace, project, issue, pk=None): + base = f"/api/v1/workspaces/{workspace.slug}/projects/{project.id}/issues/{issue.id}/comments/" + return f"{base}{pk}/" if pk else base + + @pytest.mark.django_db + def test_create_comment_notifies(self, api_key_client, workspace, project, create_issue): + with patch("plane.api.views.issue.issue_activity") as activity: + response = api_key_client.post( + self.url(workspace, project, create_issue), + {"comment_html": "

New comment

"}, + format="json", + ) + + assert response.status_code == status.HTTP_201_CREATED + kwargs = _dispatched(activity) + assert kwargs["type"] == "comment.activity.created" + assert kwargs["notification"] is True + assert kwargs["origin"] + + @pytest.mark.django_db + def test_update_comment_notifies(self, api_key_client, workspace, project, create_issue, create_comment): + with patch("plane.api.views.issue.issue_activity") as activity: + response = api_key_client.patch( + self.url(workspace, project, create_issue, create_comment.id), + {"comment_html": "

Edited comment

"}, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK + kwargs = _dispatched(activity) + assert kwargs["type"] == "comment.activity.updated" + assert kwargs["notification"] is True + assert kwargs["origin"] + + @pytest.mark.django_db + def test_delete_comment_notifies(self, api_key_client, workspace, project, create_issue, create_comment): + with patch("plane.api.views.issue.issue_activity") as activity: + response = api_key_client.delete(self.url(workspace, project, create_issue, create_comment.id)) + + assert response.status_code == status.HTTP_204_NO_CONTENT + kwargs = _dispatched(activity) + assert kwargs["type"] == "comment.activity.deleted" + assert kwargs["notification"] is True + assert kwargs["origin"] + + +@pytest.mark.contract +class TestLinkNotificationContract: + """Link writes through ``/api/v1/...`` must notify, as the web app does.""" + + def url(self, workspace, project, issue, pk=None): + base = f"/api/v1/workspaces/{workspace.slug}/projects/{project.id}/issues/{issue.id}/links/" + return f"{base}{pk}/" if pk else base + + @pytest.mark.django_db + def test_create_link_notifies(self, api_key_client, workspace, project, create_issue): + with patch("plane.api.views.issue.issue_activity") as activity: + response = api_key_client.post( + self.url(workspace, project, create_issue), + {"url": "https://example.com/spec"}, + format="json", + ) + + assert response.status_code == status.HTTP_201_CREATED + kwargs = _dispatched(activity) + assert kwargs["type"] == "link.activity.created" + assert kwargs["notification"] is True + assert kwargs["origin"] + + @pytest.mark.django_db + def test_update_link_notifies(self, api_key_client, workspace, project, create_issue, create_link): + with patch("plane.api.views.issue.issue_activity") as activity: + response = api_key_client.patch( + self.url(workspace, project, create_issue, create_link.id), + {"url": "https://example.com/updated"}, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK + kwargs = _dispatched(activity) + assert kwargs["type"] == "link.activity.updated" + assert kwargs["notification"] is True + assert kwargs["origin"] + + @pytest.mark.django_db + def test_delete_link_notifies(self, api_key_client, workspace, project, create_issue, create_link): + with patch("plane.api.views.issue.issue_activity") as activity: + response = api_key_client.delete(self.url(workspace, project, create_issue, create_link.id)) + + assert response.status_code == status.HTTP_204_NO_CONTENT + kwargs = _dispatched(activity) + assert kwargs["type"] == "link.activity.deleted" + assert kwargs["notification"] is True + assert kwargs["origin"] + + +@pytest.mark.contract +class TestIssueDeleteNotificationContract: + """#9307 covered create and update; deletion was left behind.""" + + @pytest.mark.django_db + def test_delete_issue_notifies(self, api_key_client, workspace, project, create_issue): + url = f"/api/v1/workspaces/{workspace.slug}/projects/{project.id}/issues/{create_issue.id}/" + + with patch("plane.api.views.issue.issue_activity") as activity: + response = api_key_client.delete(url) + + assert response.status_code == status.HTTP_204_NO_CONTENT + kwargs = _dispatched(activity) + assert kwargs["type"] == "issue.activity.deleted" + assert kwargs["notification"] is True + assert kwargs["origin"] From 95a81b12cc80a3875bb1daf0fccc230ca84c0620 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Wed, 19 Aug 2026 13:30:38 -0400 Subject: [PATCH 3/3] fix(api): link comment activities to the comment they describe The v1 comment create endpoint built requested_data from IssueCommentCreateSerializer, a write serializer that carries no id. So create_comment_activity stored the activity with issue_comment_id=None, and notification_task -- which gates all comment-mention extraction on that field being set -- skipped mentions entirely. The effect is that an @mention inside a comment created through the API notifies nobody, even with notification=True passed. Comment updates are unaffected, because update_comment_activity reads the id out of current_instance, which is serialized with the full IssueCommentSerializer. That asymmetry is what makes this specific to creates. Serialize the saved comment instead, matching the update path and the web app. --- apps/api/plane/api/views/issue.py | 8 ++++++- .../api/test_comment_link_notifications.py | 23 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/apps/api/plane/api/views/issue.py b/apps/api/plane/api/views/issue.py index 0155327f5a2..e51de84189b 100644 --- a/apps/api/plane/api/views/issue.py +++ b/apps/api/plane/api/views/issue.py @@ -1497,7 +1497,13 @@ def post(self, request, slug, project_id, issue_id): issue_activity.delay( type="comment.activity.created", - requested_data=json.dumps(serializer.data, cls=DjangoJSONEncoder), + # Serialize the saved comment rather than the write serializer: the + # latter carries no ``id``, so ``create_comment_activity`` stores the + # activity with ``issue_comment_id=None`` and ``notification_task`` + # then skips comment-mention extraction entirely, which is gated on + # that field. The update path already reads the id out of + # ``current_instance``, which is why only creates lose mentions. + requested_data=json.dumps(IssueCommentSerializer(issue_comment).data, cls=DjangoJSONEncoder), actor_id=str(issue_comment.created_by_id), issue_id=str(self.kwargs.get("issue_id")), project_id=str(self.kwargs.get("project_id")), diff --git a/apps/api/plane/tests/contract/api/test_comment_link_notifications.py b/apps/api/plane/tests/contract/api/test_comment_link_notifications.py index 009bcb8251d..2d4572ff87f 100644 --- a/apps/api/plane/tests/contract/api/test_comment_link_notifications.py +++ b/apps/api/plane/tests/contract/api/test_comment_link_notifications.py @@ -17,6 +17,7 @@ mocking at the dispatch boundary keeps these tests from depending on Celery. """ +import json from unittest.mock import patch import pytest @@ -109,6 +110,28 @@ def test_create_comment_notifies(self, api_key_client, workspace, project, creat assert kwargs["notification"] is True assert kwargs["origin"] + @pytest.mark.django_db + def test_create_comment_activity_carries_the_comment_id(self, api_key_client, workspace, project, create_issue): + """Without the id, notifying is not enough — @mentions are still dropped. + + ``create_comment_activity`` sets ``issue_comment_id`` from + ``requested_data["id"]``, and ``notification_task`` gates *all* + comment-mention extraction on that field being set. Serializing the write + serializer leaves it null, so a mention in an API-created comment notifies + nobody even once ``notification=True`` is passed. Comment *updates* are + unaffected, because they read the id from ``current_instance``. + """ + with patch("plane.api.views.issue.issue_activity") as activity: + response = api_key_client.post( + self.url(workspace, project, create_issue), + {"comment_html": "

Mentioning someone

"}, + format="json", + ) + + assert response.status_code == status.HTTP_201_CREATED + requested = json.loads(_dispatched(activity)["requested_data"]) + assert requested.get("id") == response.json()["id"] + @pytest.mark.django_db def test_update_comment_notifies(self, api_key_client, workspace, project, create_issue, create_comment): with patch("plane.api.views.issue.issue_activity") as activity: