mirror of
https://github.com/langgenius/dify.git
synced 2026-04-15 18:06:36 +08:00
fix: scope workflow comment reply mutations to tenant/app/comment
This commit is contained in:
parent
763524fc33
commit
5e8eeb92d9
@ -266,6 +266,9 @@ class WorkflowCommentReplyDetailApi(Resource):
|
||||
payload = WorkflowCommentReplyPayload.model_validate(console_ns.payload or {})
|
||||
|
||||
reply = WorkflowCommentService.update_reply(
|
||||
tenant_id=current_user.current_tenant_id,
|
||||
app_id=app_model.id,
|
||||
comment_id=comment_id,
|
||||
reply_id=reply_id,
|
||||
user_id=current_user.id,
|
||||
content=payload.content,
|
||||
@ -289,7 +292,13 @@ class WorkflowCommentReplyDetailApi(Resource):
|
||||
comment_id=comment_id, tenant_id=current_user.current_tenant_id, app_id=app_model.id
|
||||
)
|
||||
|
||||
WorkflowCommentService.delete_reply(reply_id=reply_id, user_id=current_user.id)
|
||||
WorkflowCommentService.delete_reply(
|
||||
tenant_id=current_user.current_tenant_id,
|
||||
app_id=app_model.id,
|
||||
comment_id=comment_id,
|
||||
reply_id=reply_id,
|
||||
user_id=current_user.id,
|
||||
)
|
||||
|
||||
return {"result": "success"}, 204
|
||||
|
||||
|
||||
@ -428,14 +428,52 @@ class WorkflowCommentService:
|
||||
return {"id": reply.id, "created_at": reply.created_at}
|
||||
|
||||
@staticmethod
|
||||
def update_reply(reply_id: str, user_id: str, content: str, mentioned_user_ids: list[str] | None = None) -> dict:
|
||||
def _get_reply_in_comment_scope(
|
||||
*,
|
||||
session: Session,
|
||||
tenant_id: str,
|
||||
app_id: str,
|
||||
comment_id: str,
|
||||
reply_id: str,
|
||||
) -> WorkflowCommentReply:
|
||||
"""Get a reply scoped to tenant/app/comment to prevent cross-thread mutations."""
|
||||
stmt = (
|
||||
select(WorkflowCommentReply)
|
||||
.join(WorkflowComment, WorkflowComment.id == WorkflowCommentReply.comment_id)
|
||||
.where(
|
||||
WorkflowCommentReply.id == reply_id,
|
||||
WorkflowCommentReply.comment_id == comment_id,
|
||||
WorkflowComment.tenant_id == tenant_id,
|
||||
WorkflowComment.app_id == app_id,
|
||||
)
|
||||
.limit(1)
|
||||
)
|
||||
reply = session.scalar(stmt)
|
||||
if not reply:
|
||||
raise NotFound("Reply not found")
|
||||
return reply
|
||||
|
||||
@staticmethod
|
||||
def update_reply(
|
||||
tenant_id: str,
|
||||
app_id: str,
|
||||
comment_id: str,
|
||||
reply_id: str,
|
||||
user_id: str,
|
||||
content: str,
|
||||
mentioned_user_ids: list[str] | None = None,
|
||||
) -> dict:
|
||||
"""Update a comment reply and notify newly mentioned users."""
|
||||
WorkflowCommentService._validate_content(content)
|
||||
|
||||
with Session(db.engine, expire_on_commit=False) as session:
|
||||
reply = session.get(WorkflowCommentReply, reply_id)
|
||||
if not reply:
|
||||
raise NotFound("Reply not found")
|
||||
reply = WorkflowCommentService._get_reply_in_comment_scope(
|
||||
session=session,
|
||||
tenant_id=tenant_id,
|
||||
app_id=app_id,
|
||||
comment_id=comment_id,
|
||||
reply_id=reply_id,
|
||||
)
|
||||
|
||||
# Only the creator can update the reply
|
||||
if reply.created_by != user_id:
|
||||
@ -488,12 +526,16 @@ class WorkflowCommentService:
|
||||
return {"id": reply.id, "updated_at": reply.updated_at}
|
||||
|
||||
@staticmethod
|
||||
def delete_reply(reply_id: str, user_id: str) -> None:
|
||||
def delete_reply(tenant_id: str, app_id: str, comment_id: str, reply_id: str, user_id: str) -> None:
|
||||
"""Delete a comment reply."""
|
||||
with Session(db.engine, expire_on_commit=False) as session:
|
||||
reply = session.get(WorkflowCommentReply, reply_id)
|
||||
if not reply:
|
||||
raise NotFound("Reply not found")
|
||||
reply = WorkflowCommentService._get_reply_in_comment_scope(
|
||||
session=session,
|
||||
tenant_id=tenant_id,
|
||||
app_id=app_id,
|
||||
comment_id=comment_id,
|
||||
reply_id=reply_id,
|
||||
)
|
||||
|
||||
# Only the creator can delete the reply
|
||||
if reply.created_by != user_id:
|
||||
|
||||
@ -395,18 +395,32 @@ class TestWorkflowCommentService:
|
||||
mock_session.commit.assert_called_once()
|
||||
|
||||
def test_update_reply_raises_not_found(self, mock_session: Mock) -> None:
|
||||
mock_session.get.return_value = None
|
||||
mock_session.scalar.return_value = None
|
||||
|
||||
with pytest.raises(NotFound):
|
||||
WorkflowCommentService.update_reply("reply-1", "user-1", "hello")
|
||||
WorkflowCommentService.update_reply(
|
||||
tenant_id="tenant-1",
|
||||
app_id="app-1",
|
||||
comment_id="comment-1",
|
||||
reply_id="reply-1",
|
||||
user_id="user-1",
|
||||
content="hello",
|
||||
)
|
||||
|
||||
def test_update_reply_raises_forbidden(self, mock_session: Mock) -> None:
|
||||
reply = Mock()
|
||||
reply.created_by = "owner"
|
||||
mock_session.get.return_value = reply
|
||||
mock_session.scalar.return_value = reply
|
||||
|
||||
with pytest.raises(Forbidden):
|
||||
WorkflowCommentService.update_reply("reply-1", "intruder", "hello")
|
||||
WorkflowCommentService.update_reply(
|
||||
tenant_id="tenant-1",
|
||||
app_id="app-1",
|
||||
comment_id="comment-1",
|
||||
reply_id="reply-1",
|
||||
user_id="intruder",
|
||||
content="hello",
|
||||
)
|
||||
|
||||
def test_update_reply_replaces_mentions(self, mock_session: Mock) -> None:
|
||||
reply = Mock()
|
||||
@ -414,11 +428,18 @@ class TestWorkflowCommentService:
|
||||
reply.comment_id = "comment-1"
|
||||
reply.created_by = "owner"
|
||||
reply.updated_at = "updated"
|
||||
mock_session.get.return_value = reply
|
||||
mock_session.scalar.return_value = reply
|
||||
mock_session.scalars.return_value = _mock_scalars([Mock()])
|
||||
comment = Mock()
|
||||
comment.tenant_id = "tenant-1"
|
||||
comment.app_id = "app-1"
|
||||
mock_session.get.return_value = comment
|
||||
|
||||
with patch.object(WorkflowCommentService, "_filter_valid_mentioned_user_ids", return_value=["user-2"]):
|
||||
result = WorkflowCommentService.update_reply(
|
||||
tenant_id="tenant-1",
|
||||
app_id="app-1",
|
||||
comment_id="comment-1",
|
||||
reply_id="reply-1",
|
||||
user_id="owner",
|
||||
content="new",
|
||||
@ -457,24 +478,42 @@ class TestWorkflowCommentService:
|
||||
def test_delete_reply_raises_forbidden(self, mock_session: Mock) -> None:
|
||||
reply = Mock()
|
||||
reply.created_by = "owner"
|
||||
mock_session.get.return_value = reply
|
||||
mock_session.scalar.return_value = reply
|
||||
|
||||
with pytest.raises(Forbidden):
|
||||
WorkflowCommentService.delete_reply("reply-1", "intruder")
|
||||
WorkflowCommentService.delete_reply(
|
||||
tenant_id="tenant-1",
|
||||
app_id="app-1",
|
||||
comment_id="comment-1",
|
||||
reply_id="reply-1",
|
||||
user_id="intruder",
|
||||
)
|
||||
|
||||
def test_delete_reply_raises_not_found(self, mock_session: Mock) -> None:
|
||||
mock_session.get.return_value = None
|
||||
mock_session.scalar.return_value = None
|
||||
|
||||
with pytest.raises(NotFound):
|
||||
WorkflowCommentService.delete_reply("reply-1", "owner")
|
||||
WorkflowCommentService.delete_reply(
|
||||
tenant_id="tenant-1",
|
||||
app_id="app-1",
|
||||
comment_id="comment-1",
|
||||
reply_id="reply-1",
|
||||
user_id="owner",
|
||||
)
|
||||
|
||||
def test_delete_reply_removes_mentions(self, mock_session: Mock) -> None:
|
||||
reply = Mock()
|
||||
reply.created_by = "owner"
|
||||
mock_session.get.return_value = reply
|
||||
mock_session.scalar.return_value = reply
|
||||
mock_session.scalars.return_value = _mock_scalars([Mock(), Mock()])
|
||||
|
||||
WorkflowCommentService.delete_reply("reply-1", "owner")
|
||||
WorkflowCommentService.delete_reply(
|
||||
tenant_id="tenant-1",
|
||||
app_id="app-1",
|
||||
comment_id="comment-1",
|
||||
reply_id="reply-1",
|
||||
user_id="owner",
|
||||
)
|
||||
|
||||
assert mock_session.delete.call_count == 3
|
||||
mock_session.commit.assert_called_once()
|
||||
|
||||
Loading…
Reference in New Issue
Block a user