From 3cf56c8662d2b027c2b1721f867f36fcfc3b71d7 Mon Sep 17 00:00:00 2001 From: hjlarry Date: Mon, 13 Apr 2026 09:21:47 +0800 Subject: [PATCH] preserve comment mentions on position updates --- .../console/app/workflow_comment.py | 4 + api/services/workflow_comment_service.py | 75 ++++++++++--------- .../console/app/test_workflow_comment_api.py | 29 +++++++ .../services/test_workflow_comment_service.py | 51 +++++++++++++ 4 files changed, 125 insertions(+), 34 deletions(-) diff --git a/api/controllers/console/app/workflow_comment.py b/api/controllers/console/app/workflow_comment.py index bdbf998f65..c10b697d90 100644 --- a/api/controllers/console/app/workflow_comment.py +++ b/api/controllers/console/app/workflow_comment.py @@ -39,6 +39,10 @@ class WorkflowCommentCreatePayload(WorkflowCommentContentPayload): class WorkflowCommentUpdatePayload(WorkflowCommentContentPayload): position_x: float | None = Field(default=None, description="Comment X position") position_y: float | None = Field(default=None, description="Comment Y position") + mentioned_user_ids: list[str] | None = Field( + default=None, + description="Mentioned user IDs. Omit to keep existing mentions.", + ) class WorkflowCommentReplyPayload(WorkflowCommentContentPayload): diff --git a/api/services/workflow_comment_service.py b/api/services/workflow_comment_service.py index 3a59456f9f..ff47e4f253 100644 --- a/api/services/workflow_comment_service.py +++ b/api/services/workflow_comment_service.py @@ -274,7 +274,11 @@ class WorkflowCommentService: position_y: float | None = None, mentioned_user_ids: list[str] | None = None, ) -> dict: - """Update a workflow comment and notify newly mentioned users.""" + """Update a workflow comment and notify newly mentioned users. + + `mentioned_user_ids=None` means "leave mentions unchanged". + Passing an explicit list replaces the existing comment mentions, including clearing them with `[]`. + """ WorkflowCommentService._validate_content(content) with Session(db.engine, expire_on_commit=False) as session: @@ -300,42 +304,45 @@ class WorkflowCommentService: if position_y is not None: comment.position_y = position_y - # Update mentions - first remove existing mentions for this comment only (not replies) - existing_mentions = session.scalars( - select(WorkflowCommentMention).where( - WorkflowCommentMention.comment_id == comment.id, - WorkflowCommentMention.reply_id.is_(None), # Only comment mentions, not reply mentions - ) - ).all() - existing_mentioned_user_ids = {mention.mentioned_user_id for mention in existing_mentions} - for mention in existing_mentions: - session.delete(mention) + mention_email_payloads: list[dict[str, str]] = [] + if mentioned_user_ids is not None: + # Replace comment mentions only when the client explicitly sends the mention list. + existing_mentions = session.scalars( + select(WorkflowCommentMention).where( + WorkflowCommentMention.comment_id == comment.id, + WorkflowCommentMention.reply_id.is_(None), # Only comment mentions, not reply mentions + ) + ).all() + existing_mentioned_user_ids = {mention.mentioned_user_id for mention in existing_mentions} + for mention in existing_mentions: + session.delete(mention) - # Add new mentions - mentioned_user_ids = WorkflowCommentService._filter_valid_mentioned_user_ids( - mentioned_user_ids or [], - session=session, - tenant_id=tenant_id, - ) - new_mentioned_user_ids = [ - user_id for user_id in mentioned_user_ids if user_id not in existing_mentioned_user_ids - ] - for user_id_str in mentioned_user_ids: - mention = WorkflowCommentMention( - comment_id=comment.id, - reply_id=None, # This is a comment mention - mentioned_user_id=user_id_str, + filtered_mentioned_user_ids = WorkflowCommentService._filter_valid_mentioned_user_ids( + mentioned_user_ids, + session=session, + tenant_id=tenant_id, ) - session.add(mention) + new_mentioned_user_ids = [ + mentioned_user_id + for mentioned_user_id in filtered_mentioned_user_ids + if mentioned_user_id not in existing_mentioned_user_ids + ] + for mentioned_user_id in filtered_mentioned_user_ids: + mention = WorkflowCommentMention( + comment_id=comment.id, + reply_id=None, # This is a comment mention + mentioned_user_id=mentioned_user_id, + ) + session.add(mention) - mention_email_payloads = WorkflowCommentService._build_mention_email_payloads( - session=session, - tenant_id=tenant_id, - app_id=app_id, - mentioner_id=user_id, - mentioned_user_ids=new_mentioned_user_ids, - content=content, - ) + mention_email_payloads = WorkflowCommentService._build_mention_email_payloads( + session=session, + tenant_id=tenant_id, + app_id=app_id, + mentioner_id=user_id, + mentioned_user_ids=new_mentioned_user_ids, + content=content, + ) session.commit() WorkflowCommentService._dispatch_mention_emails(mention_email_payloads) diff --git a/api/tests/unit_tests/controllers/console/app/test_workflow_comment_api.py b/api/tests/unit_tests/controllers/console/app/test_workflow_comment_api.py index 41b4aff671..0c1a9b38a4 100644 --- a/api/tests/unit_tests/controllers/console/app/test_workflow_comment_api.py +++ b/api/tests/unit_tests/controllers/console/app/test_workflow_comment_api.py @@ -2,6 +2,7 @@ from __future__ import annotations from contextlib import nullcontext from dataclasses import dataclass +from datetime import datetime from types import SimpleNamespace from unittest.mock import MagicMock, PropertyMock, patch @@ -172,3 +173,31 @@ def test_create_comment_allows_editor(app: Flask, monkeypatch: pytest.MonkeyPatc position_y=2.0, mentioned_user_ids=[], ) + + +def test_update_comment_omits_mentions_when_payload_does_not_include_them( + app: Flask, monkeypatch: pytest.MonkeyPatch +) -> None: + app.config.setdefault("RESTX_MASK_HEADER", "X-Fields") + account = _make_account(TenantAccountRole.EDITOR) + app_model = _make_app() + _patch_console_guards(monkeypatch, account, app_model) + + update_comment_mock = MagicMock(return_value={"id": "comment-1", "updated_at": datetime(2024, 1, 1, 12, 0, 0)}) + monkeypatch.setattr(workflow_comment_module.WorkflowCommentService, "update_comment", update_comment_mock) + payload = {"content": "hello", "position_x": 10.0, "position_y": 20.0} + + with app.test_request_context("/console/api/apps/app-123/workflow/comments/comment-1", method="PUT", json=payload): + with _patch_payload(payload): + workflow_comment_module.WorkflowCommentDetailApi().put(app_id="app-123", comment_id="comment-1") + + update_comment_mock.assert_called_once_with( + tenant_id="tenant-123", + app_id="app-123", + comment_id="comment-1", + user_id="account-123", + content="hello", + position_x=10.0, + position_y=20.0, + mentioned_user_ids=None, + ) diff --git a/api/tests/unit_tests/services/test_workflow_comment_service.py b/api/tests/unit_tests/services/test_workflow_comment_service.py index 1352ed840c..e6db068e07 100644 --- a/api/tests/unit_tests/services/test_workflow_comment_service.py +++ b/api/tests/unit_tests/services/test_workflow_comment_service.py @@ -215,6 +215,57 @@ class TestWorkflowCommentService: assert mock_session.add.call_count == 1 mock_session.commit.assert_called_once() + def test_update_comment_preserves_mentions_when_mentioned_user_ids_omitted(self, mock_session: Mock) -> None: + comment = Mock() + comment.id = "comment-1" + comment.created_by = "owner" + mock_session.scalar.return_value = comment + + with ( + patch.object(WorkflowCommentService, "_filter_valid_mentioned_user_ids") as filter_mentions_mock, + patch.object(WorkflowCommentService, "_build_mention_email_payloads") as build_payloads_mock, + patch.object(WorkflowCommentService, "_dispatch_mention_emails") as dispatch_mock, + ): + result = WorkflowCommentService.update_comment( + tenant_id="tenant-1", + app_id="app-1", + comment_id="comment-1", + user_id="owner", + content="updated", + ) + + assert result == {"id": "comment-1", "updated_at": comment.updated_at} + mock_session.delete.assert_not_called() + mock_session.add.assert_not_called() + filter_mentions_mock.assert_not_called() + build_payloads_mock.assert_not_called() + dispatch_mock.assert_called_once_with([]) + mock_session.commit.assert_called_once() + + def test_update_comment_clears_mentions_when_empty_list_provided(self, mock_session: Mock) -> None: + comment = Mock() + comment.id = "comment-1" + comment.created_by = "owner" + mock_session.scalar.return_value = comment + + existing_mentions = [Mock(), Mock()] + mock_session.scalars.return_value = _mock_scalars(existing_mentions) + + with patch.object(WorkflowCommentService, "_filter_valid_mentioned_user_ids", return_value=[]): + result = WorkflowCommentService.update_comment( + tenant_id="tenant-1", + app_id="app-1", + comment_id="comment-1", + user_id="owner", + content="updated", + mentioned_user_ids=[], + ) + + assert result == {"id": "comment-1", "updated_at": comment.updated_at} + assert mock_session.delete.call_count == 2 + mock_session.add.assert_not_called() + mock_session.commit.assert_called_once() + def test_update_comment_notifies_only_new_mentions(self, mock_session: Mock) -> None: comment = Mock() comment.id = "comment-1"