mirror of
https://github.com/langgenius/dify.git
synced 2026-04-15 18:06:36 +08:00
preserve comment mentions on position updates
This commit is contained in:
parent
e7de5919a0
commit
3cf56c8662
@ -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):
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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,
|
||||
)
|
||||
|
||||
@ -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"
|
||||
|
||||
Loading…
Reference in New Issue
Block a user