From 890a2a62cf11e718a746da65cdc8ef7da4fd7ad7 Mon Sep 17 00:00:00 2001 From: Chiranjeevi Balawat <2371852+jeevi@users.noreply.github.com> Date: Mon, 12 Oct 2026 02:25:17 +0000 Subject: [PATCH] refactor(api): decouple console workflow comments (#43508) --- api/.importlinter | 30 + api/controllers/common/errors.py | 6 + .../console/app/workflow_comment.py | 400 +++------ api/extensions/ext_application_services.py | 9 + api/models/comment.py | 85 +- .../app/workflow_comment_repository.py | 434 ++++++++++ .../app/workflow_comment_mention_gateway.py | 25 + api/services/app/workflow_comment_service.py | 449 ++++++++++ api/services/workflow_comment_service.py | 564 ------------- api/tests/unit_tests/.ruff.toml | 1 - .../console/app/test_workflow_comment_api.py | 765 ++++++++--------- .../test_ext_application_services.py | 19 + .../unit_tests/models/test_comment_models.py | 131 --- api/tests/unit_tests/pyrefly.toml | 2 - .../app/test_workflow_comment_repository.py | 766 ++++++++++++++++++ .../app/test_workflow_comment_service.py | 319 ++++++++ .../services/test_workflow_comment_service.py | 703 ---------------- 17 files changed, 2566 insertions(+), 2142 deletions(-) create mode 100644 api/repositories/app/workflow_comment_repository.py create mode 100644 api/services/app/workflow_comment_mention_gateway.py create mode 100644 api/services/app/workflow_comment_service.py delete mode 100644 api/services/workflow_comment_service.py delete mode 100644 api/tests/unit_tests/models/test_comment_models.py create mode 100644 api/tests/unit_tests/repositories/app/test_workflow_comment_repository.py create mode 100644 api/tests/unit_tests/services/app/test_workflow_comment_service.py delete mode 100644 api/tests/unit_tests/services/test_workflow_comment_service.py diff --git a/api/.importlinter b/api/.importlinter index 2550486a174..79daf2b84df 100644 --- a/api/.importlinter +++ b/api/.importlinter @@ -1064,6 +1064,36 @@ forbidden_modules = # so only direct imports are checked here. allow_indirect_imports = True +[importlinter:contract:workflow-comment-service-boundary] +name = Workflow comment application service is framework and persistence neutral +type = forbidden +source_modules = + services.app.workflow_comment_service +forbidden_modules = + configs + controllers + extensions + flask + models + repositories + sqlalchemy + tasks + werkzeug + +[importlinter:contract:console-workflow-comment-controller-boundary] +name = Console workflow comment controller reaches persistence only through the composition root +type = forbidden +source_modules = + controllers.console.app.workflow_comment +forbidden_modules = + extensions.ext_database + models + repositories + sqlalchemy +# The composition root (extensions.ext_application_services) wires repositories, +# so only direct imports are checked here. +allow_indirect_imports = True + [importlinter:contract:console-app-wraps-boundary] name = Console app decorators load App rows without reaching into repositories type = forbidden diff --git a/api/controllers/common/errors.py b/api/controllers/common/errors.py index aa8316d6103..05a3fd22389 100644 --- a/api/controllers/common/errors.py +++ b/api/controllers/common/errors.py @@ -98,6 +98,12 @@ class UnauthorizedError(BaseHTTPException): description = "Authentication is required." +class ForbiddenError(BaseHTTPException): + error_code = "forbidden" + code = HTTPStatus.FORBIDDEN + description = "You don't have the permission to access the requested resource." + + class InternalServerError(BaseHTTPException): """Expose a safe response while retaining the original exception in server logs.""" diff --git a/api/controllers/console/app/workflow_comment.py b/api/controllers/console/app/workflow_comment.py index c70c44fd4f4..6506581fa29 100644 --- a/api/controllers/console/app/workflow_comment.py +++ b/api/controllers/console/app/workflow_comment.py @@ -1,40 +1,37 @@ -import logging +from collections.abc import Generator +from contextlib import contextmanager from datetime import datetime +from uuid import UUID from flask_restx import Resource -from pydantic import BaseModel, Field, TypeAdapter, computed_field, field_validator -from sqlalchemy.orm import Session +from pydantic import BaseModel, Field, computed_field, field_validator +from controllers.common.errors import ForbiddenError, InvalidArgumentError, NotFoundError from controllers.common.schema import register_response_schema_models, register_schema_models from controllers.console import console_ns -from controllers.console.app.wraps import get_app_model -from controllers.console.wraps import ( - account_initialization_required, - edit_permission_required, - model_validate, - setup_required, - with_current_tenant_id, - with_current_user, -) +from controllers.console.app.error import AppNotFoundError +from controllers.console.flask_admission import console_account_admission +from controllers.console.wraps import model_validate +from enums.account import TenantAccountRole from extensions.ext_application_services import application_services -from extensions.ext_database import db from fields.base import ResponseModel from fields.member_fields import AccountWithRole from libs.helper import build_avatar_url, dump_response, to_timestamp -from libs.login import login_required -from models import Account, App -from models.comment import ( - WorkflowComment as WorkflowCommentModel, +from machinery.context import RequestContext +from services.app.workflow_comment_service import ( + InvalidMentionedUserIdError, + InvalidWorkflowCommentContentError, + WorkflowCommentAppNotFoundError, + WorkflowCommentDraft, + WorkflowCommentEdit, + WorkflowCommentNotFoundError, + WorkflowCommentPermissionError, + WorkflowCommentReplyDraft, + WorkflowCommentReplyNotFoundError, ) -from models.comment import ( - WorkflowCommentMention as WorkflowCommentMentionModel, -) -from models.comment import ( - WorkflowCommentReply as WorkflowCommentReplyModel, -) -from services.workflow_comment_service import WorkflowCommentService -logger = logging.getLogger(__name__) +# Same roles as the legacy `edit_permission_required` gate; like it, admission skips the role check under RBAC. +_EDIT_ROLES = frozenset({TenantAccountRole.OWNER, TenantAccountRole.ADMIN, TenantAccountRole.EDITOR}) class WorkflowCommentCreatePayload(BaseModel): @@ -143,73 +140,6 @@ class WorkflowCommentDetail(ResponseModel): return to_timestamp(value) -def _workflow_comment_reply_response(reply: WorkflowCommentReplyModel, *, session: Session) -> WorkflowCommentReply: - return WorkflowCommentReply.model_validate( - { - "id": reply.id, - "content": reply.content, - "created_by": reply.created_by, - "created_by_account": reply.created_by_account(session), - "created_at": reply.created_at, - } - ) - - -def _workflow_comment_mention_response( - mention: WorkflowCommentMentionModel, *, session: Session -) -> WorkflowCommentMention: - return WorkflowCommentMention.model_validate( - { - "mentioned_user_id": mention.mentioned_user_id, - "mentioned_user_account": mention.mentioned_user_account(session), - "reply_id": mention.reply_id, - } - ) - - -def _workflow_comment_basic_response(comment: WorkflowCommentModel, *, session: Session) -> WorkflowCommentBasic: - return WorkflowCommentBasic.model_validate( - { - "id": comment.id, - "position_x": comment.position_x, - "position_y": comment.position_y, - "content": comment.content, - "created_by": comment.created_by, - "created_by_account": comment.created_by_account(session), - "created_at": comment.created_at, - "updated_at": comment.updated_at, - "resolved": comment.resolved, - "resolved_at": comment.resolved_at, - "resolved_by": comment.resolved_by, - "resolved_by_account": comment.resolved_by_account(session), - "reply_count": comment.reply_count, - "mention_count": comment.mention_count, - "participants": comment.participants(session), - } - ) - - -def _workflow_comment_detail_response(comment: WorkflowCommentModel, *, session: Session) -> WorkflowCommentDetail: - return WorkflowCommentDetail.model_validate( - { - "id": comment.id, - "position_x": comment.position_x, - "position_y": comment.position_y, - "content": comment.content, - "created_by": comment.created_by, - "created_by_account": comment.created_by_account(session), - "created_at": comment.created_at, - "updated_at": comment.updated_at, - "resolved": comment.resolved, - "resolved_at": comment.resolved_at, - "resolved_by": comment.resolved_by, - "resolved_by_account": comment.resolved_by_account(session), - "replies": [_workflow_comment_reply_response(reply, session=session) for reply in comment.replies], - "mentions": [_workflow_comment_mention_response(mention, session=session) for mention in comment.mentions], - } - ) - - class WorkflowCommentCreate(ResponseModel): id: str created_at: int | None = None @@ -286,6 +216,21 @@ register_response_schema_models( ) +@contextmanager +def _translate_errors() -> Generator[None]: + """Map framework-neutral comment failures back to the existing transport errors.""" + try: + yield + except WorkflowCommentAppNotFoundError as error: + raise AppNotFoundError() from error + except (WorkflowCommentNotFoundError, WorkflowCommentReplyNotFoundError) as error: + raise NotFoundError(str(error)) from error + except WorkflowCommentPermissionError as error: + raise ForbiddenError(str(error)) from error + except (InvalidWorkflowCommentContentError, InvalidMentionedUserIdError) as error: + raise InvalidArgumentError(str(error)) from error + + @console_ns.route("/apps//workflow/comments") class WorkflowCommentListApi(Resource): """API for listing and creating workflow comments.""" @@ -294,52 +239,33 @@ class WorkflowCommentListApi(Resource): @console_ns.doc(description="Get all comments for a workflow") @console_ns.doc(params={"app_id": "Application ID"}) @console_ns.response(200, "Comments retrieved successfully", console_ns.models[WorkflowCommentBasicList.__name__]) - @login_required - @setup_required - @account_initialization_required - @with_current_tenant_id - @get_app_model() - def get(self, current_tenant_id: str, app_model: App): + @console_account_admission() + def get(self, request_context: RequestContext, app_id: UUID): """Get all comments for a workflow.""" - comments = WorkflowCommentService.get_comments(tenant_id=current_tenant_id, app_id=app_model.id) - session = db.session() - - return WorkflowCommentBasicList.model_validate( - {"data": [_workflow_comment_basic_response(comment, session=session) for comment in comments]} - ).model_dump(mode="json") + with _translate_errors(): + comments = application_services().workflow_comments.list_comments(request_context, str(app_id)) + return dump_response(WorkflowCommentBasicList, {"data": comments}) @console_ns.doc("create_workflow_comment") @console_ns.doc(description="Create a new workflow comment") @console_ns.doc(params={"app_id": "Application ID"}) @console_ns.expect(console_ns.models[WorkflowCommentCreatePayload.__name__]) @console_ns.response(201, "Comment created successfully", console_ns.models[WorkflowCommentCreate.__name__]) - @login_required - @setup_required - @account_initialization_required - @edit_permission_required - @with_current_user - @with_current_tenant_id - @get_app_model() + @console_account_admission(allowed_roles=_EDIT_ROLES) @model_validate(WorkflowCommentCreatePayload) - def post( - self, - req_data: WorkflowCommentCreatePayload, - current_tenant_id: str, - current_user: Account, - app_model: App, - ): + def post(self, req_data: WorkflowCommentCreatePayload, request_context: RequestContext, app_id: UUID): """Create a new workflow comment.""" - - result = WorkflowCommentService.create_comment( - tenant_id=current_tenant_id, - app_id=app_model.id, - created_by=current_user.id, - content=req_data.content, - position_x=req_data.position_x, - position_y=req_data.position_y, - mentioned_user_ids=req_data.mentioned_user_ids, - ) - + with _translate_errors(): + result = application_services().workflow_comments.create_comment( + request_context, + str(app_id), + WorkflowCommentDraft( + content=req_data.content, + position_x=req_data.position_x, + position_y=req_data.position_y, + mentioned_user_ids=tuple(req_data.mentioned_user_ids), + ), + ) return dump_response(WorkflowCommentCreate, result), 201 @@ -351,76 +277,52 @@ class WorkflowCommentDetailApi(Resource): @console_ns.doc(description="Get a specific workflow comment") @console_ns.doc(params={"app_id": "Application ID", "comment_id": "Comment ID"}) @console_ns.response(200, "Comment retrieved successfully", console_ns.models[WorkflowCommentDetail.__name__]) - @login_required - @setup_required - @account_initialization_required - @with_current_tenant_id - @get_app_model() - def get(self, current_tenant_id: str, app_model: App, comment_id: str): + @console_account_admission() + def get(self, request_context: RequestContext, app_id: UUID, comment_id: str): """Get a specific workflow comment.""" - comment = WorkflowCommentService.get_comment( - tenant_id=current_tenant_id, app_id=app_model.id, comment_id=comment_id - ) - session = db.session() - - return dump_response(WorkflowCommentDetail, _workflow_comment_detail_response(comment, session=session)) + with _translate_errors(): + comment = application_services().workflow_comments.get_comment(request_context, str(app_id), comment_id) + return dump_response(WorkflowCommentDetail, comment) @console_ns.doc("update_workflow_comment") @console_ns.doc(description="Update a workflow comment") @console_ns.doc(params={"app_id": "Application ID", "comment_id": "Comment ID"}) @console_ns.expect(console_ns.models[WorkflowCommentUpdatePayload.__name__]) @console_ns.response(200, "Comment updated successfully", console_ns.models[WorkflowCommentUpdate.__name__]) - @login_required - @setup_required - @account_initialization_required - @edit_permission_required - @with_current_user - @with_current_tenant_id - @get_app_model() + @console_account_admission(allowed_roles=_EDIT_ROLES) @model_validate(WorkflowCommentUpdatePayload) def put( self, req_data: WorkflowCommentUpdatePayload, - current_tenant_id: str, - current_user: Account, - app_model: App, + request_context: RequestContext, + app_id: UUID, comment_id: str, ): """Update a workflow comment.""" - - result = WorkflowCommentService.update_comment( - tenant_id=current_tenant_id, - app_id=app_model.id, - comment_id=comment_id, - user_id=current_user.id, - content=req_data.content, - position_x=req_data.position_x, - position_y=req_data.position_y, - mentioned_user_ids=req_data.mentioned_user_ids, - ) - + mentioned_user_ids = req_data.mentioned_user_ids + with _translate_errors(): + result = application_services().workflow_comments.update_comment( + request_context, + str(app_id), + comment_id, + WorkflowCommentEdit( + content=req_data.content, + position_x=req_data.position_x, + position_y=req_data.position_y, + mentioned_user_ids=tuple(mentioned_user_ids) if mentioned_user_ids is not None else None, + ), + ) return dump_response(WorkflowCommentUpdate, result) @console_ns.doc("delete_workflow_comment") @console_ns.doc(description="Delete a workflow comment") @console_ns.doc(params={"app_id": "Application ID", "comment_id": "Comment ID"}) @console_ns.response(204, "Comment deleted successfully") - @login_required - @setup_required - @account_initialization_required - @edit_permission_required - @with_current_user - @with_current_tenant_id - @get_app_model() - def delete(self, current_tenant_id: str, current_user: Account, app_model: App, comment_id: str): + @console_account_admission(allowed_roles=_EDIT_ROLES) + def delete(self, request_context: RequestContext, app_id: UUID, comment_id: str): """Delete a workflow comment.""" - WorkflowCommentService.delete_comment( - tenant_id=current_tenant_id, - app_id=app_model.id, - comment_id=comment_id, - user_id=current_user.id, - ) - + with _translate_errors(): + application_services().workflow_comments.delete_comment(request_context, str(app_id), comment_id) return "", 204 @@ -432,23 +334,12 @@ class WorkflowCommentResolveApi(Resource): @console_ns.doc(description="Resolve a workflow comment") @console_ns.doc(params={"app_id": "Application ID", "comment_id": "Comment ID"}) @console_ns.response(200, "Comment resolved successfully", console_ns.models[WorkflowCommentResolve.__name__]) - @login_required - @setup_required - @account_initialization_required - @edit_permission_required - @with_current_user - @with_current_tenant_id - @get_app_model() - def post(self, current_tenant_id: str, current_user: Account, app_model: App, comment_id: str): + @console_account_admission(allowed_roles=_EDIT_ROLES) + def post(self, request_context: RequestContext, app_id: UUID, comment_id: str): """Resolve a workflow comment.""" - comment = WorkflowCommentService.resolve_comment( - tenant_id=current_tenant_id, - app_id=app_model.id, - comment_id=comment_id, - user_id=current_user.id, - ) - - return dump_response(WorkflowCommentResolve, comment) + with _translate_errors(): + result = application_services().workflow_comments.resolve_comment(request_context, str(app_id), comment_id) + return dump_response(WorkflowCommentResolve, result) @console_ns.route("/apps//workflow/comments//replies") @@ -460,35 +351,26 @@ class WorkflowCommentReplyApi(Resource): @console_ns.doc(params={"app_id": "Application ID", "comment_id": "Comment ID"}) @console_ns.expect(console_ns.models[WorkflowCommentReplyPayload.__name__]) @console_ns.response(201, "Reply created successfully", console_ns.models[WorkflowCommentReplyCreate.__name__]) - @login_required - @setup_required - @account_initialization_required - @edit_permission_required - @with_current_user - @with_current_tenant_id - @get_app_model() + @console_account_admission(allowed_roles=_EDIT_ROLES) @model_validate(WorkflowCommentReplyPayload) def post( self, req_data: WorkflowCommentReplyPayload, - current_tenant_id: str, - current_user: Account, - app_model: App, + request_context: RequestContext, + app_id: UUID, comment_id: str, ): """Add a reply to a workflow comment.""" - # Validate comment access first - WorkflowCommentService.validate_comment_access( - comment_id=comment_id, tenant_id=current_tenant_id, app_id=app_model.id - ) - - result = WorkflowCommentService.create_reply( - comment_id=comment_id, - content=req_data.content, - created_by=current_user.id, - mentioned_user_ids=req_data.mentioned_user_ids, - ) - + with _translate_errors(): + result = application_services().workflow_comments.create_reply( + request_context, + str(app_id), + comment_id, + WorkflowCommentReplyDraft( + content=req_data.content, + mentioned_user_ids=tuple(req_data.mentioned_user_ids), + ), + ) return dump_response(WorkflowCommentReplyCreate, result), 201 @@ -501,67 +383,39 @@ class WorkflowCommentReplyDetailApi(Resource): @console_ns.doc(params={"app_id": "Application ID", "comment_id": "Comment ID", "reply_id": "Reply ID"}) @console_ns.expect(console_ns.models[WorkflowCommentReplyPayload.__name__]) @console_ns.response(200, "Reply updated successfully", console_ns.models[WorkflowCommentReplyUpdate.__name__]) - @login_required - @setup_required - @account_initialization_required - @edit_permission_required - @with_current_user - @with_current_tenant_id - @get_app_model() + @console_account_admission(allowed_roles=_EDIT_ROLES) @model_validate(WorkflowCommentReplyPayload) def put( self, req_data: WorkflowCommentReplyPayload, - current_tenant_id: str, - current_user: Account, - app_model: App, + request_context: RequestContext, + app_id: UUID, comment_id: str, reply_id: str, ): """Update a comment reply.""" - # Validate comment access first - WorkflowCommentService.validate_comment_access( - comment_id=comment_id, tenant_id=current_tenant_id, app_id=app_model.id - ) - - reply = WorkflowCommentService.update_reply( - tenant_id=current_tenant_id, - app_id=app_model.id, - comment_id=comment_id, - reply_id=reply_id, - user_id=current_user.id, - content=req_data.content, - mentioned_user_ids=req_data.mentioned_user_ids, - ) - - return dump_response(WorkflowCommentReplyUpdate, reply) + with _translate_errors(): + result = application_services().workflow_comments.update_reply( + request_context, + str(app_id), + comment_id, + reply_id, + WorkflowCommentReplyDraft( + content=req_data.content, + mentioned_user_ids=tuple(req_data.mentioned_user_ids), + ), + ) + return dump_response(WorkflowCommentReplyUpdate, result) @console_ns.doc("delete_workflow_comment_reply") @console_ns.doc(description="Delete a comment reply") @console_ns.doc(params={"app_id": "Application ID", "comment_id": "Comment ID", "reply_id": "Reply ID"}) @console_ns.response(204, "Reply deleted successfully") - @login_required - @setup_required - @account_initialization_required - @edit_permission_required - @with_current_user - @with_current_tenant_id - @get_app_model() - def delete(self, current_tenant_id: str, current_user: Account, app_model: App, comment_id: str, reply_id: str): + @console_account_admission(allowed_roles=_EDIT_ROLES) + def delete(self, request_context: RequestContext, app_id: UUID, comment_id: str, reply_id: str): """Delete a comment reply.""" - # Validate comment access first - WorkflowCommentService.validate_comment_access( - comment_id=comment_id, tenant_id=current_tenant_id, app_id=app_model.id - ) - - WorkflowCommentService.delete_reply( - tenant_id=current_tenant_id, - app_id=app_model.id, - comment_id=comment_id, - reply_id=reply_id, - user_id=current_user.id, - ) - + with _translate_errors(): + application_services().workflow_comments.delete_reply(request_context, str(app_id), comment_id, reply_id) return "", 204 @@ -575,17 +429,11 @@ class WorkflowCommentMentionUsersApi(Resource): @console_ns.response( 200, "Mentionable users retrieved successfully", console_ns.models[WorkflowCommentMentionUsersPayload.__name__] ) - @login_required - @setup_required - @account_initialization_required - @with_current_user - @get_app_model() - def get(self, current_user: Account, app_model: App): + @console_account_admission() + def get(self, request_context: RequestContext, app_id: UUID): """Get all users in current tenant for mentions.""" - current_tenant = current_user.current_tenant # need the tenant object here - if current_tenant is None: - raise ValueError("current tenant is required") - members = application_services().workspaces.member_queries.list_members(current_tenant.id) - users = TypeAdapter(list[AccountWithRole]).validate_python(members, from_attributes=True) - response = WorkflowCommentMentionUsersPayload(users=users) - return response.model_dump(mode="json"), 200 + services = application_services() + with _translate_errors(): + services.workflow_comments.ensure_app(request_context, str(app_id)) + members = services.workspaces.member_queries.list_members(request_context.active_workspace_id) + return dump_response(WorkflowCommentMentionUsersPayload, {"users": members}), 200 diff --git a/api/extensions/ext_application_services.py b/api/extensions/ext_application_services.py index f9f951159d2..64cb036c57e 100644 --- a/api/extensions/ext_application_services.py +++ b/api/extensions/ext_application_services.py @@ -56,6 +56,7 @@ from repositories.app.console_repository import ConsoleAppRepository from repositories.app.mcp_server_repository import AppMCPServerRepository from repositories.app.site_command_repository import AppSiteCommandRepository from repositories.app.tracing_config_repository import SQLAlchemyAppTracingConfigRepository +from repositories.app.workflow_comment_repository import WorkflowCommentRepository from repositories.app_definition_query_repository import AppDefinitionQueryRepository from repositories.app_preview_query_repository import AppPreviewQueryRepository from repositories.app_scoped_end_user_repository import AppScopedEndUserRepo @@ -101,6 +102,8 @@ from services.api_based_extension_application_service import APIBasedExtensionAp from services.app.advanced_prompt_template_service import AdvancedPromptTemplateService from services.app.api_key_service import AppApiKeyService from services.app.mcp_server_service import AppMCPServerService +from services.app.workflow_comment_mention_gateway import CeleryWorkflowCommentMentionNotifier +from services.app.workflow_comment_service import WorkflowCommentService from services.app_audio_adapters import AppAudioRuntime from services.app_audio_service import AppAudio from services.app_definition_query_service import AppDefinitionQueryService @@ -298,6 +301,7 @@ class ApplicationServices: workflow_runs: WorkflowRunService workspaces: WorkspaceServices workflow_app_logs: WorkflowAppLogQueryService + workflow_comments: WorkflowCommentService inner_mail: InnerMailService web_passport: WebPassportService tags: TagApplicationService @@ -679,6 +683,11 @@ def build_application_services( workflow_app_logs=WorkflowAppLogQueryService( logs=WorkflowAppLogQueryRepository(session_factory=database_client), ), + workflow_comments=WorkflowCommentService( + comments=WorkflowCommentRepository(session_factory=database_client), + notifier=CeleryWorkflowCommentMentionNotifier(), + console_web_url=dify_config.CONSOLE_WEB_URL, + ), inner_mail=InnerMailService(dispatch=enqueue_inner_mail), web_passport=WebPassportService( passports=WebPassportRepository( diff --git a/api/models/comment.py b/api/models/comment.py index cdf1087a01a..4dd09f8b729 100644 --- a/api/models/comment.py +++ b/api/models/comment.py @@ -6,11 +6,10 @@ from datetime import datetime import sqlalchemy as sa from sqlalchemy import Index, func -from sqlalchemy.orm import Mapped, Session, mapped_column, relationship +from sqlalchemy.orm import Mapped, mapped_column, relationship from models.base import TypeBase -from .account import Account from .base import gen_uuidv7_string from .types import StringUUID @@ -73,68 +72,6 @@ class WorkflowComment(TypeBase): lambda: WorkflowCommentMention, back_populates="comment", cascade="all, delete-orphan", init=False ) - def created_by_account(self, session: Session) -> Account | None: - """Get creator account.""" - if hasattr(self, "_created_by_account_cache"): - return self._created_by_account_cache - return session.get(Account, self.created_by) - - def cache_created_by_account(self, account: Account | None) -> None: - """Cache creator account to avoid extra queries.""" - self._created_by_account_cache = account - - def resolved_by_account(self, session: Session) -> Account | None: - """Get resolver account.""" - if hasattr(self, "_resolved_by_account_cache"): - return self._resolved_by_account_cache - if self.resolved_by: - return session.get(Account, self.resolved_by) - return None - - def cache_resolved_by_account(self, account: Account | None) -> None: - """Cache resolver account to avoid extra queries.""" - self._resolved_by_account_cache = account - - @property - def reply_count(self): - """Get reply count.""" - return len(self.replies) - - @property - def mention_count(self): - """Get mention count.""" - return len(self.mentions) - - def participants(self, session: Session) -> list[Account]: - """Get all participants (creator + repliers + mentioned users).""" - participant_ids: set[str] = set() - participants: list[Account] = [] - - # Use account accessors to reuse preloaded caches and avoid hidden N+1. - if self.created_by not in participant_ids: - participant_ids.add(self.created_by) - created_by_account = self.created_by_account(session) - if created_by_account: - participants.append(created_by_account) - - for reply in self.replies: - if reply.created_by in participant_ids: - continue - participant_ids.add(reply.created_by) - reply_account = reply.created_by_account(session) - if reply_account: - participants.append(reply_account) - - for mention in self.mentions: - if mention.mentioned_user_id in participant_ids: - continue - participant_ids.add(mention.mentioned_user_id) - mentioned_account = mention.mentioned_user_account(session) - if mentioned_account: - participants.append(mentioned_account) - - return participants - class WorkflowCommentReply(TypeBase): """Workflow comment reply model. @@ -173,16 +110,6 @@ class WorkflowCommentReply(TypeBase): # Relationships comment: Mapped[WorkflowComment] = relationship(lambda: WorkflowComment, back_populates="replies", init=False) - def created_by_account(self, session: Session) -> Account | None: - """Get creator account.""" - if hasattr(self, "_created_by_account_cache"): - return self._created_by_account_cache - return session.get(Account, self.created_by) - - def cache_created_by_account(self, account: Account | None) -> None: - """Cache creator account to avoid extra queries.""" - self._created_by_account_cache = account - class WorkflowCommentMention(TypeBase): """Workflow comment mention model. @@ -216,13 +143,3 @@ class WorkflowCommentMention(TypeBase): # Relationships comment: Mapped[WorkflowComment] = relationship(lambda: WorkflowComment, back_populates="mentions", init=False) reply: Mapped[WorkflowCommentReply | None] = relationship(lambda: WorkflowCommentReply, init=False) - - def mentioned_user_account(self, session: Session) -> Account | None: - """Get mentioned account.""" - if hasattr(self, "_mentioned_user_account_cache"): - return self._mentioned_user_account_cache - return session.get(Account, self.mentioned_user_id) - - def cache_mentioned_user_account(self, account: Account | None) -> None: - """Cache mentioned account to avoid extra queries.""" - self._mentioned_user_account_cache = account diff --git a/api/repositories/app/workflow_comment_repository.py b/api/repositories/app/workflow_comment_repository.py new file mode 100644 index 00000000000..df2342933ac --- /dev/null +++ b/api/repositories/app/workflow_comment_repository.py @@ -0,0 +1,434 @@ +"""SQLAlchemy persistence adapter for Console workflow comments.""" + +from collections.abc import Collection, Iterable, Mapping, Sequence +from typing import override + +from sqlalchemy import delete, desc, select +from sqlalchemy.orm import Session, selectinload, sessionmaker + +from libs.datetime_utils import naive_utc_now +from models.account import Account, TenantAccountJoin +from models.comment import WorkflowComment, WorkflowCommentMention, WorkflowCommentReply +from models.enums import AppStatus +from models.model import App +from repositories.app.console_visibility import console_visible_condition +from services.app.workflow_comment_service import ( + MentionContext, + MentionRecipient, + WorkflowCommentAccount, + WorkflowCommentCreated, + WorkflowCommentDetail, + WorkflowCommentDraft, + WorkflowCommentEdit, + WorkflowCommentMentionRecord, + WorkflowCommentNotFoundError, + WorkflowCommentPermissionError, + WorkflowCommentReplyDraft, + WorkflowCommentReplyNotFoundError, + WorkflowCommentReplyRecord, + WorkflowCommentResolved, + WorkflowCommentStore, + WorkflowCommentSummary, + WorkflowCommentUpdated, + WorkflowCommentWrite, +) + +_NO_MENTIONS = MentionContext(app_name=None, commenter_name=None, recipients=()) + + +class WorkflowCommentRepository(WorkflowCommentStore): + def __init__(self, *, session_factory: sessionmaker[Session]) -> None: + self._session_factory = session_factory + + @override + def app_exists(self, *, workspace_id: str, app_id: str) -> bool: + with self._session_factory() as session: + found = session.scalar( + select(App.id) + .where( + App.id == app_id, + App.tenant_id == workspace_id, + App.status == AppStatus.NORMAL, + console_visible_condition(), + ) + .limit(1) + ) + return found is not None + + @override + def list_comments(self, *, workspace_id: str, app_id: str) -> tuple[WorkflowCommentSummary, ...]: + with self._session_factory() as session: + comments = session.scalars( + select(WorkflowComment) + .options(selectinload(WorkflowComment.replies), selectinload(WorkflowComment.mentions)) + .where(WorkflowComment.tenant_id == workspace_id, WorkflowComment.app_id == app_id) + .order_by(desc(WorkflowComment.created_at)) + ).all() + accounts = self._load_accounts(session, comments) + return tuple(self._summary(comment, accounts) for comment in comments) + + @override + def get_comment(self, *, workspace_id: str, app_id: str, comment_id: str) -> WorkflowCommentDetail: + with self._session_factory() as session: + comment = session.scalar( + select(WorkflowComment) + .options(selectinload(WorkflowComment.replies), selectinload(WorkflowComment.mentions)) + .where( + WorkflowComment.id == comment_id, + WorkflowComment.tenant_id == workspace_id, + WorkflowComment.app_id == app_id, + ) + .limit(1) + ) + if comment is None: + raise WorkflowCommentNotFoundError + accounts = self._load_accounts(session, [comment]) + return self._detail(comment, accounts) + + @override + def insert_comment( + self, *, workspace_id: str, app_id: str, actor_id: str, draft: WorkflowCommentDraft + ) -> WorkflowCommentWrite[WorkflowCommentCreated]: + with self._session_factory.begin() as session: + comment = WorkflowComment( + tenant_id=workspace_id, + app_id=app_id, + position_x=draft.position_x, + position_y=draft.position_y, + content=draft.content, + created_by=actor_id, + ) + session.add(comment) + session.flush() + + mentioned_user_ids = self._workspace_member_ids(session, workspace_id, draft.mentioned_user_ids) + session.add_all( + WorkflowCommentMention(comment_id=comment.id, reply_id=None, mentioned_user_id=user_id) + for user_id in mentioned_user_ids + ) + mentions = self._mention_context(session, workspace_id, app_id, actor_id, mentioned_user_ids) + session.flush() + session.refresh(comment, ["created_at"]) + return WorkflowCommentWrite( + result=WorkflowCommentCreated(id=comment.id, created_at=comment.created_at), + mentions=mentions, + ) + + @override + def update_comment( + self, *, workspace_id: str, app_id: str, comment_id: str, actor_id: str, edit: WorkflowCommentEdit + ) -> WorkflowCommentWrite[WorkflowCommentUpdated]: + with self._session_factory.begin() as session: + comment = self._require_comment(session, workspace_id, app_id, comment_id) + if comment.created_by != actor_id: + raise WorkflowCommentPermissionError("Only the comment creator can update it") + + comment.content = edit.content + if edit.position_x is not None: + comment.position_x = edit.position_x + if edit.position_y is not None: + comment.position_y = edit.position_y + + mentions = _NO_MENTIONS + if edit.mentioned_user_ids is not None: + # Replace the comment's own mentions only when the client sends a mention list. + existing_mentions = session.scalars( + select(WorkflowCommentMention).where( + WorkflowCommentMention.comment_id == comment.id, + WorkflowCommentMention.reply_id.is_(None), + ) + ).all() + previously_mentioned = {mention.mentioned_user_id for mention in existing_mentions} + for mention in existing_mentions: + session.delete(mention) + + mentioned_user_ids = self._workspace_member_ids(session, workspace_id, edit.mentioned_user_ids) + session.add_all( + WorkflowCommentMention(comment_id=comment.id, reply_id=None, mentioned_user_id=user_id) + for user_id in mentioned_user_ids + ) + mentions = self._mention_context( + session, + workspace_id, + app_id, + actor_id, + [user_id for user_id in mentioned_user_ids if user_id not in previously_mentioned], + ) + + session.flush() + session.refresh(comment, ["updated_at"]) + return WorkflowCommentWrite( + result=WorkflowCommentUpdated(id=comment.id, updated_at=comment.updated_at), + mentions=mentions, + ) + + @override + def delete_comment(self, *, workspace_id: str, app_id: str, comment_id: str, actor_id: str) -> None: + with self._session_factory.begin() as session: + comment = self._require_comment(session, workspace_id, app_id, comment_id) + if comment.created_by != actor_id: + raise WorkflowCommentPermissionError("Only the comment creator can delete it") + + session.execute(delete(WorkflowCommentMention).where(WorkflowCommentMention.comment_id == comment.id)) + session.execute(delete(WorkflowCommentReply).where(WorkflowCommentReply.comment_id == comment.id)) + session.delete(comment) + + @override + def resolve_comment( + self, *, workspace_id: str, app_id: str, comment_id: str, actor_id: str + ) -> WorkflowCommentResolved: + with self._session_factory.begin() as session: + comment = self._require_comment(session, workspace_id, app_id, comment_id) + if not comment.resolved: + comment.resolved = True + comment.resolved_at = naive_utc_now() + comment.resolved_by = actor_id + return WorkflowCommentResolved( + id=comment.id, + resolved=comment.resolved, + resolved_at=comment.resolved_at, + resolved_by=comment.resolved_by, + ) + + @override + def insert_reply( + self, *, workspace_id: str, app_id: str, comment_id: str, actor_id: str, draft: WorkflowCommentReplyDraft + ) -> WorkflowCommentWrite[WorkflowCommentCreated]: + with self._session_factory.begin() as session: + comment = self._require_comment(session, workspace_id, app_id, comment_id) + reply = WorkflowCommentReply(comment_id=comment.id, content=draft.content, created_by=actor_id) + session.add(reply) + session.flush() + + mentioned_user_ids = self._workspace_member_ids(session, workspace_id, draft.mentioned_user_ids) + session.add_all( + WorkflowCommentMention(comment_id=comment.id, reply_id=reply.id, mentioned_user_id=user_id) + for user_id in mentioned_user_ids + ) + mentions = self._mention_context(session, workspace_id, app_id, actor_id, mentioned_user_ids) + session.flush() + session.refresh(reply, ["created_at"]) + return WorkflowCommentWrite( + result=WorkflowCommentCreated(id=reply.id, created_at=reply.created_at), + mentions=mentions, + ) + + @override + def update_reply( + self, + *, + workspace_id: str, + app_id: str, + comment_id: str, + reply_id: str, + actor_id: str, + draft: WorkflowCommentReplyDraft, + ) -> WorkflowCommentWrite[WorkflowCommentUpdated]: + with self._session_factory.begin() as session: + reply = self._require_reply(session, workspace_id, app_id, comment_id, reply_id) + if reply.created_by != actor_id: + raise WorkflowCommentPermissionError("Only the reply creator can update it") + + reply.content = draft.content + existing_mentions = session.scalars( + select(WorkflowCommentMention).where(WorkflowCommentMention.reply_id == reply.id) + ).all() + previously_mentioned = {mention.mentioned_user_id for mention in existing_mentions} + for mention in existing_mentions: + session.delete(mention) + + mentioned_user_ids = self._workspace_member_ids(session, workspace_id, draft.mentioned_user_ids) + session.add_all( + WorkflowCommentMention(comment_id=reply.comment_id, reply_id=reply.id, mentioned_user_id=user_id) + for user_id in mentioned_user_ids + ) + mentions = self._mention_context( + session, + workspace_id, + app_id, + actor_id, + [user_id for user_id in mentioned_user_ids if user_id not in previously_mentioned], + ) + session.flush() + session.refresh(reply, ["updated_at"]) + return WorkflowCommentWrite( + result=WorkflowCommentUpdated(id=reply.id, updated_at=reply.updated_at), + mentions=mentions, + ) + + @override + def delete_reply(self, *, workspace_id: str, app_id: str, comment_id: str, reply_id: str, actor_id: str) -> None: + with self._session_factory.begin() as session: + reply = self._require_reply(session, workspace_id, app_id, comment_id, reply_id) + if reply.created_by != actor_id: + raise WorkflowCommentPermissionError("Only the reply creator can delete it") + + session.execute(delete(WorkflowCommentMention).where(WorkflowCommentMention.reply_id == reply.id)) + session.delete(reply) + + @staticmethod + def _require_comment(session: Session, workspace_id: str, app_id: str, comment_id: str) -> WorkflowComment: + comment = session.scalar( + select(WorkflowComment) + .where( + WorkflowComment.id == comment_id, + WorkflowComment.tenant_id == workspace_id, + WorkflowComment.app_id == app_id, + ) + .limit(1) + ) + if comment is None: + raise WorkflowCommentNotFoundError + return comment + + @classmethod + def _require_reply( + cls, session: Session, workspace_id: str, app_id: str, comment_id: str, reply_id: str + ) -> WorkflowCommentReply: + """Require the reply inside its comment's scope; a missing comment is reported before a missing reply.""" + cls._require_comment(session, workspace_id, app_id, comment_id) + reply = session.scalar( + select(WorkflowCommentReply) + .where(WorkflowCommentReply.id == reply_id, WorkflowCommentReply.comment_id == comment_id) + .limit(1) + ) + if reply is None: + raise WorkflowCommentReplyNotFoundError + return reply + + @staticmethod + def _workspace_member_ids(session: Session, workspace_id: str, user_ids: Sequence[str]) -> list[str]: + """Keep only IDs of workspace members, preserving the caller's order.""" + if not user_ids: + return [] + member_ids = { + str(account_id) + for account_id in session.scalars( + select(TenantAccountJoin.account_id).where( + TenantAccountJoin.tenant_id == workspace_id, + TenantAccountJoin.account_id.in_(user_ids), + ) + ).all() + } + return [user_id for user_id in user_ids if user_id in member_ids] + + @staticmethod + def _mention_context( + session: Session, workspace_id: str, app_id: str, actor_id: str, mentioned_user_ids: Collection[str] + ) -> MentionContext: + recipient_ids = [user_id for user_id in mentioned_user_ids if user_id != actor_id] + if not recipient_ids: + return _NO_MENTIONS + + accounts = session.scalars( + select(Account) + .join(TenantAccountJoin, TenantAccountJoin.account_id == Account.id) + .where(TenantAccountJoin.tenant_id == workspace_id, Account.id.in_(recipient_ids)) + ).all() + return MentionContext( + app_name=session.scalar(select(App.name).where(App.id == app_id, App.tenant_id == workspace_id)), + commenter_name=session.scalar(select(Account.name).where(Account.id == actor_id)), + recipients=tuple( + MentionRecipient( + email=account.email, + name=account.name, + interface_language=account.interface_language, + ) + for account in accounts + if account.email + ), + ) + + @staticmethod + def _load_accounts(session: Session, comments: Iterable[WorkflowComment]) -> dict[str, WorkflowCommentAccount]: + """Batch-load every account referenced by the comments, their replies, and mentions.""" + user_ids: set[str] = set() + for comment in comments: + user_ids.add(comment.created_by) + if comment.resolved_by: + user_ids.add(comment.resolved_by) + user_ids.update(reply.created_by for reply in comment.replies) + user_ids.update(mention.mentioned_user_id for mention in comment.mentions) + if not user_ids: + return {} + + return { + str(account.id): WorkflowCommentAccount( + id=str(account.id), + name=account.name, + email=account.email, + avatar=account.avatar, + ) + for account in session.scalars(select(Account).where(Account.id.in_(user_ids))).all() + } + + @classmethod + def _summary( + cls, comment: WorkflowComment, accounts: Mapping[str, WorkflowCommentAccount] + ) -> WorkflowCommentSummary: + return WorkflowCommentSummary( + id=comment.id, + position_x=comment.position_x, + position_y=comment.position_y, + content=comment.content, + created_by=comment.created_by, + created_by_account=accounts.get(comment.created_by), + created_at=comment.created_at, + updated_at=comment.updated_at, + resolved=comment.resolved, + resolved_at=comment.resolved_at, + resolved_by=comment.resolved_by, + resolved_by_account=accounts.get(comment.resolved_by) if comment.resolved_by else None, + reply_count=len(comment.replies), + mention_count=len(comment.mentions), + participants=cls._participants(comment, accounts), + ) + + @staticmethod + def _detail(comment: WorkflowComment, accounts: Mapping[str, WorkflowCommentAccount]) -> WorkflowCommentDetail: + return WorkflowCommentDetail( + id=comment.id, + position_x=comment.position_x, + position_y=comment.position_y, + content=comment.content, + created_by=comment.created_by, + created_by_account=accounts.get(comment.created_by), + created_at=comment.created_at, + updated_at=comment.updated_at, + resolved=comment.resolved, + resolved_at=comment.resolved_at, + resolved_by=comment.resolved_by, + resolved_by_account=accounts.get(comment.resolved_by) if comment.resolved_by else None, + replies=tuple( + WorkflowCommentReplyRecord( + id=reply.id, + content=reply.content, + created_by=reply.created_by, + created_by_account=accounts.get(reply.created_by), + created_at=reply.created_at, + ) + for reply in comment.replies + ), + mentions=tuple( + WorkflowCommentMentionRecord( + mentioned_user_id=mention.mentioned_user_id, + mentioned_user_account=accounts.get(mention.mentioned_user_id), + reply_id=mention.reply_id, + ) + for mention in comment.mentions + ), + ) + + @staticmethod + def _participants( + comment: WorkflowComment, accounts: Mapping[str, WorkflowCommentAccount] + ) -> tuple[WorkflowCommentAccount, ...]: + """Return the creator, repliers, then mentioned users, each once and in first-seen order.""" + participant_ids = dict.fromkeys( + [ + comment.created_by, + *(reply.created_by for reply in comment.replies), + *(mention.mentioned_user_id for mention in comment.mentions), + ] + ) + return tuple(accounts[user_id] for user_id in participant_ids if user_id in accounts) diff --git a/api/services/app/workflow_comment_mention_gateway.py b/api/services/app/workflow_comment_mention_gateway.py new file mode 100644 index 00000000000..dc2d5d95d0d --- /dev/null +++ b/api/services/app/workflow_comment_mention_gateway.py @@ -0,0 +1,25 @@ +"""Celery adapter that delivers workflow comment mention notifications by email.""" + +from collections.abc import Sequence +from typing import override + +from services.app.workflow_comment_service import ( + WorkflowCommentMentionNotification, + WorkflowCommentMentionNotifier, +) +from tasks.mail_workflow_comment_task import send_workflow_comment_mention_email_task + + +class CeleryWorkflowCommentMentionNotifier(WorkflowCommentMentionNotifier): + @override + def notify(self, notifications: Sequence[WorkflowCommentMentionNotification]) -> None: + for notification in notifications: + send_workflow_comment_mention_email_task.delay( + language=notification.language, + to=notification.to, + mentioned_name=notification.mentioned_name, + commenter_name=notification.commenter_name, + app_name=notification.app_name, + comment_content=notification.comment_content, + app_url=notification.app_url, + ) diff --git a/api/services/app/workflow_comment_service.py b/api/services/app/workflow_comment_service.py new file mode 100644 index 00000000000..69dfdf71be7 --- /dev/null +++ b/api/services/app/workflow_comment_service.py @@ -0,0 +1,449 @@ +"""Application boundary for Console workflow canvas comments. + +Comments belong to an app rather than to a workflow version. Every read and write is scoped by the complete +workspace -> app -> comment -> reply owner chain, and only the creator of a comment or reply may edit or delete it. +Mention notifications are dispatched only after the store has committed the write. +""" + +import uuid +from collections.abc import Sequence +from dataclasses import dataclass +from datetime import datetime +from typing import Protocol + +from machinery.context import RequestContext + +MAX_COMMENT_CONTENT_LENGTH = 1000 +MENTION_EXCERPT_LENGTH = 200 + + +@dataclass(frozen=True, slots=True) +class WorkflowCommentAccount: + id: str + name: str + email: str + avatar: str | None + + +@dataclass(frozen=True, slots=True) +class WorkflowCommentReplyRecord: + id: str + content: str + created_by: str + created_by_account: WorkflowCommentAccount | None + created_at: datetime + + +@dataclass(frozen=True, slots=True) +class WorkflowCommentMentionRecord: + mentioned_user_id: str + mentioned_user_account: WorkflowCommentAccount | None + reply_id: str | None + + +@dataclass(frozen=True, slots=True) +class WorkflowCommentSummary: + """A comment as listed on the canvas, with counts and participants instead of full threads.""" + + id: str + position_x: float + position_y: float + content: str + created_by: str + created_by_account: WorkflowCommentAccount | None + created_at: datetime + updated_at: datetime + resolved: bool + resolved_at: datetime | None + resolved_by: str | None + resolved_by_account: WorkflowCommentAccount | None + reply_count: int + mention_count: int + participants: tuple[WorkflowCommentAccount, ...] + + +@dataclass(frozen=True, slots=True) +class WorkflowCommentDetail: + id: str + position_x: float + position_y: float + content: str + created_by: str + created_by_account: WorkflowCommentAccount | None + created_at: datetime + updated_at: datetime + resolved: bool + resolved_at: datetime | None + resolved_by: str | None + resolved_by_account: WorkflowCommentAccount | None + replies: tuple[WorkflowCommentReplyRecord, ...] + mentions: tuple[WorkflowCommentMentionRecord, ...] + + +@dataclass(frozen=True, slots=True) +class WorkflowCommentCreated: + id: str + created_at: datetime + + +@dataclass(frozen=True, slots=True) +class WorkflowCommentUpdated: + id: str + updated_at: datetime + + +@dataclass(frozen=True, slots=True) +class WorkflowCommentResolved: + id: str + resolved: bool + resolved_at: datetime | None + resolved_by: str | None + + +@dataclass(frozen=True, slots=True) +class WorkflowCommentDraft: + content: str + position_x: float + position_y: float + mentioned_user_ids: tuple[str, ...] + + +@dataclass(frozen=True, slots=True) +class WorkflowCommentEdit: + """Comment edits; None positions are kept, and None mentions leave the comment's mentions untouched.""" + + content: str + position_x: float | None + position_y: float | None + mentioned_user_ids: tuple[str, ...] | None + + +@dataclass(frozen=True, slots=True) +class WorkflowCommentReplyDraft: + """Reply content; the mention list always replaces the reply's existing mentions.""" + + content: str + mentioned_user_ids: tuple[str, ...] + + +@dataclass(frozen=True, slots=True) +class MentionRecipient: + """A workspace member to notify, with raw profile values; the service applies display fallbacks.""" + + email: str + name: str | None + interface_language: str | None + + +@dataclass(frozen=True, slots=True) +class MentionContext: + """Values read inside the write transaction for building mention notifications.""" + + app_name: str | None + commenter_name: str | None + recipients: tuple[MentionRecipient, ...] + + +@dataclass(frozen=True, slots=True) +class WorkflowCommentWrite[T]: + result: T + mentions: MentionContext + + +@dataclass(frozen=True, slots=True) +class WorkflowCommentMentionNotification: + language: str + to: str + mentioned_name: str + commenter_name: str + app_name: str + comment_content: str + app_url: str + + +class WorkflowCommentStore(Protocol): + """Persistence for workflow comments, always scoped by workspace and app. + + Write methods run in one bounded transaction. They raise `WorkflowCommentNotFoundError` or + `WorkflowCommentReplyNotFoundError` when the target is outside the scope, and + `WorkflowCommentPermissionError` when `actor_id` did not create it. Mention IDs are already + normalized; the store keeps only workspace members and reports recipients for newly mentioned + members other than the actor. + """ + + def app_exists(self, *, workspace_id: str, app_id: str) -> bool: + """Return whether Console may address the app (normal status, not a hidden backing app).""" + ... + + def list_comments(self, *, workspace_id: str, app_id: str) -> Sequence[WorkflowCommentSummary]: + """Return the app's comments, newest first.""" + ... + + def get_comment(self, *, workspace_id: str, app_id: str, comment_id: str) -> WorkflowCommentDetail: ... + + def insert_comment( + self, *, workspace_id: str, app_id: str, actor_id: str, draft: WorkflowCommentDraft + ) -> WorkflowCommentWrite[WorkflowCommentCreated]: ... + + def update_comment( + self, *, workspace_id: str, app_id: str, comment_id: str, actor_id: str, edit: WorkflowCommentEdit + ) -> WorkflowCommentWrite[WorkflowCommentUpdated]: ... + + def delete_comment(self, *, workspace_id: str, app_id: str, comment_id: str, actor_id: str) -> None: + """Delete the comment with its replies and mentions.""" + ... + + def resolve_comment( + self, *, workspace_id: str, app_id: str, comment_id: str, actor_id: str + ) -> WorkflowCommentResolved: + """Mark the comment resolved by any member; an already resolved comment is returned unchanged.""" + ... + + def insert_reply( + self, *, workspace_id: str, app_id: str, comment_id: str, actor_id: str, draft: WorkflowCommentReplyDraft + ) -> WorkflowCommentWrite[WorkflowCommentCreated]: ... + + def update_reply( + self, + *, + workspace_id: str, + app_id: str, + comment_id: str, + reply_id: str, + actor_id: str, + draft: WorkflowCommentReplyDraft, + ) -> WorkflowCommentWrite[WorkflowCommentUpdated]: ... + + def delete_reply(self, *, workspace_id: str, app_id: str, comment_id: str, reply_id: str, actor_id: str) -> None: + """Delete the reply with its mentions.""" + ... + + +class WorkflowCommentMentionNotifier(Protocol): + def notify(self, notifications: Sequence[WorkflowCommentMentionNotification]) -> None: ... + + +class WorkflowCommentError(Exception): + """Base class for framework-neutral workflow comment failures.""" + + +class WorkflowCommentAppNotFoundError(WorkflowCommentError): + def __init__(self) -> None: + super().__init__("App not found") + + +class WorkflowCommentNotFoundError(WorkflowCommentError): + def __init__(self) -> None: + super().__init__("Comment not found") + + +class WorkflowCommentReplyNotFoundError(WorkflowCommentError): + def __init__(self) -> None: + super().__init__("Reply not found") + + +class WorkflowCommentPermissionError(WorkflowCommentError): + """Raised when an account edits or deletes a comment or reply it did not create.""" + + +class InvalidWorkflowCommentContentError(WorkflowCommentError): + pass + + +class InvalidMentionedUserIdError(WorkflowCommentError): + def __init__(self, user_id: str) -> None: + super().__init__(f"{user_id} is not a valid uuid.") + + +class WorkflowCommentService: + def __init__( + self, + *, + comments: WorkflowCommentStore, + notifier: WorkflowCommentMentionNotifier, + console_web_url: str, + ) -> None: + self._comments = comments + self._notifier = notifier + self._console_web_url = console_web_url.rstrip("/") + + def ensure_app(self, context: RequestContext, app_id: str) -> None: + if not self._comments.app_exists(workspace_id=context.active_workspace_id, app_id=app_id): + raise WorkflowCommentAppNotFoundError + + def list_comments(self, context: RequestContext, app_id: str) -> tuple[WorkflowCommentSummary, ...]: + self.ensure_app(context, app_id) + return tuple(self._comments.list_comments(workspace_id=context.active_workspace_id, app_id=app_id)) + + def get_comment(self, context: RequestContext, app_id: str, comment_id: str) -> WorkflowCommentDetail: + self.ensure_app(context, app_id) + return self._comments.get_comment( + workspace_id=context.active_workspace_id, app_id=app_id, comment_id=comment_id + ) + + def create_comment( + self, context: RequestContext, app_id: str, draft: WorkflowCommentDraft + ) -> WorkflowCommentCreated: + self.ensure_app(context, app_id) + _validate_content(draft.content) + write = self._comments.insert_comment( + workspace_id=context.active_workspace_id, + app_id=app_id, + actor_id=context.account_id, + draft=WorkflowCommentDraft( + content=draft.content, + position_x=draft.position_x, + position_y=draft.position_y, + mentioned_user_ids=normalize_mentioned_user_ids(draft.mentioned_user_ids), + ), + ) + self._notify(app_id, draft.content, write.mentions) + return write.result + + def update_comment( + self, context: RequestContext, app_id: str, comment_id: str, edit: WorkflowCommentEdit + ) -> WorkflowCommentUpdated: + self.ensure_app(context, app_id) + _validate_content(edit.content) + mentioned_user_ids = edit.mentioned_user_ids + write = self._comments.update_comment( + workspace_id=context.active_workspace_id, + app_id=app_id, + comment_id=comment_id, + actor_id=context.account_id, + edit=WorkflowCommentEdit( + content=edit.content, + position_x=edit.position_x, + position_y=edit.position_y, + mentioned_user_ids=( + normalize_mentioned_user_ids(mentioned_user_ids) if mentioned_user_ids is not None else None + ), + ), + ) + self._notify(app_id, edit.content, write.mentions) + return write.result + + def delete_comment(self, context: RequestContext, app_id: str, comment_id: str) -> None: + self.ensure_app(context, app_id) + self._comments.delete_comment( + workspace_id=context.active_workspace_id, + app_id=app_id, + comment_id=comment_id, + actor_id=context.account_id, + ) + + def resolve_comment(self, context: RequestContext, app_id: str, comment_id: str) -> WorkflowCommentResolved: + self.ensure_app(context, app_id) + return self._comments.resolve_comment( + workspace_id=context.active_workspace_id, + app_id=app_id, + comment_id=comment_id, + actor_id=context.account_id, + ) + + def create_reply( + self, context: RequestContext, app_id: str, comment_id: str, draft: WorkflowCommentReplyDraft + ) -> WorkflowCommentCreated: + self.ensure_app(context, app_id) + _validate_content(draft.content) + write = self._comments.insert_reply( + workspace_id=context.active_workspace_id, + app_id=app_id, + comment_id=comment_id, + actor_id=context.account_id, + draft=WorkflowCommentReplyDraft( + content=draft.content, + mentioned_user_ids=normalize_mentioned_user_ids(draft.mentioned_user_ids), + ), + ) + self._notify(app_id, draft.content, write.mentions) + return write.result + + def update_reply( + self, + context: RequestContext, + app_id: str, + comment_id: str, + reply_id: str, + draft: WorkflowCommentReplyDraft, + ) -> WorkflowCommentUpdated: + self.ensure_app(context, app_id) + _validate_content(draft.content) + write = self._comments.update_reply( + workspace_id=context.active_workspace_id, + app_id=app_id, + comment_id=comment_id, + reply_id=reply_id, + actor_id=context.account_id, + draft=WorkflowCommentReplyDraft( + content=draft.content, + mentioned_user_ids=normalize_mentioned_user_ids(draft.mentioned_user_ids), + ), + ) + self._notify(app_id, draft.content, write.mentions) + return write.result + + def delete_reply(self, context: RequestContext, app_id: str, comment_id: str, reply_id: str) -> None: + self.ensure_app(context, app_id) + self._comments.delete_reply( + workspace_id=context.active_workspace_id, + app_id=app_id, + comment_id=comment_id, + reply_id=reply_id, + actor_id=context.account_id, + ) + + def _notify(self, app_id: str, content: str, mentions: MentionContext) -> None: + if not mentions.recipients: + return + app_name = mentions.app_name or "Dify app" + commenter_name = mentions.commenter_name or "Dify user" + excerpt = format_comment_excerpt(content) + app_url = f"{self._console_web_url}/app/{app_id}/workflow" + self._notifier.notify( + [ + WorkflowCommentMentionNotification( + language=recipient.interface_language or "en-US", + to=recipient.email, + mentioned_name=recipient.name or recipient.email, + commenter_name=commenter_name, + app_name=app_name, + comment_content=excerpt, + app_url=app_url, + ) + for recipient in mentions.recipients + ] + ) + + +def normalize_mentioned_user_ids(mentioned_user_ids: Sequence[str]) -> tuple[str, ...]: + """Return deduplicated user IDs in input order, skipping empty IDs and rejecting malformed ones.""" + unique_user_ids: dict[str, None] = {} + for user_id in mentioned_user_ids: + if not user_id: + continue + try: + uuid.UUID(user_id) + except ValueError as error: + raise InvalidMentionedUserIdError(user_id) from error + unique_user_ids.setdefault(user_id, None) + return tuple(unique_user_ids) + + +def format_comment_excerpt(content: str, max_length: int = MENTION_EXCERPT_LENGTH) -> str: + """Trim comment content for email display.""" + trimmed = content.strip() + if len(trimmed) <= max_length: + return trimmed + if max_length <= 3: + return trimmed[:max_length] + return f"{trimmed[: max_length - 3].rstrip()}..." + + +def _validate_content(content: str) -> None: + if len(content.strip()) == 0: + raise InvalidWorkflowCommentContentError("Comment content cannot be empty") + if len(content) > MAX_COMMENT_CONTENT_LENGTH: + raise InvalidWorkflowCommentContentError( + f"Comment content cannot exceed {MAX_COMMENT_CONTENT_LENGTH} characters" + ) diff --git a/api/services/workflow_comment_service.py b/api/services/workflow_comment_service.py deleted file mode 100644 index ff47e4f253e..00000000000 --- a/api/services/workflow_comment_service.py +++ /dev/null @@ -1,564 +0,0 @@ -import logging -from collections.abc import Sequence - -from sqlalchemy import desc, select -from sqlalchemy.orm import Session, selectinload -from werkzeug.exceptions import Forbidden, NotFound - -from configs import dify_config -from extensions.ext_database import db -from libs.datetime_utils import naive_utc_now -from libs.helper import uuid_value -from models import App, TenantAccountJoin, WorkflowComment, WorkflowCommentMention, WorkflowCommentReply -from models.account import Account -from tasks.mail_workflow_comment_task import send_workflow_comment_mention_email_task - -logger = logging.getLogger(__name__) - - -class WorkflowCommentService: - """Service for managing workflow comments.""" - - @staticmethod - def _validate_content(content: str) -> None: - if len(content.strip()) == 0: - raise ValueError("Comment content cannot be empty") - - if len(content) > 1000: - raise ValueError("Comment content cannot exceed 1000 characters") - - @staticmethod - def _filter_valid_mentioned_user_ids( - mentioned_user_ids: Sequence[str], *, session: Session, tenant_id: str - ) -> list[str]: - """Return deduplicated UUID user IDs that belong to the tenant, preserving input order.""" - unique_user_ids: list[str] = [] - seen: set[str] = set() - for user_id in mentioned_user_ids: - if not isinstance(user_id, str): - continue - if not uuid_value(user_id): - continue - if user_id in seen: - continue - seen.add(user_id) - unique_user_ids.append(user_id) - if not unique_user_ids: - return [] - - tenant_member_ids = { - str(account_id) - for account_id in session.scalars( - select(TenantAccountJoin.account_id).where( - TenantAccountJoin.tenant_id == tenant_id, - TenantAccountJoin.account_id.in_(unique_user_ids), - ) - ).all() - } - - return [user_id for user_id in unique_user_ids if user_id in tenant_member_ids] - - @staticmethod - def _format_comment_excerpt(content: str, max_length: int = 200) -> str: - """Trim comment content for email display.""" - trimmed = content.strip() - if len(trimmed) <= max_length: - return trimmed - if max_length <= 3: - return trimmed[:max_length] - return f"{trimmed[: max_length - 3].rstrip()}..." - - @staticmethod - def _build_mention_email_payloads( - session: Session, - tenant_id: str, - app_id: str, - mentioner_id: str, - mentioned_user_ids: Sequence[str], - content: str, - ) -> list[dict[str, str]]: - """Prepare email payloads for mentioned users, including workflow app link.""" - if not mentioned_user_ids: - return [] - - candidate_user_ids = [user_id for user_id in mentioned_user_ids if user_id != mentioner_id] - if not candidate_user_ids: - return [] - - app_name_value = session.scalar(select(App.name).where(App.id == app_id, App.tenant_id == tenant_id)) - app_name = app_name_value if isinstance(app_name_value, str) and app_name_value else "Dify app" - commenter_name_value = session.scalar(select(Account.name).where(Account.id == mentioner_id)) - commenter_name = ( - commenter_name_value if isinstance(commenter_name_value, str) and commenter_name_value else "Dify user" - ) - comment_excerpt = WorkflowCommentService._format_comment_excerpt(content) - base_url = dify_config.CONSOLE_WEB_URL.rstrip("/") - app_url = f"{base_url}/app/{app_id}/workflow" - - accounts = session.scalars( - select(Account) - .join(TenantAccountJoin, TenantAccountJoin.account_id == Account.id) - .where(TenantAccountJoin.tenant_id == tenant_id, Account.id.in_(candidate_user_ids)) - ).all() - - payloads: list[dict[str, str]] = [] - for account in accounts: - email = account.email - if not isinstance(email, str) or not email: - continue - mentioned_name = account.name if isinstance(account.name, str) and account.name else email - language = ( - account.interface_language - if isinstance(account.interface_language, str) and account.interface_language - else "en-US" - ) - payloads.append( - { - "language": language, - "to": email, - "mentioned_name": mentioned_name, - "commenter_name": commenter_name, - "app_name": app_name, - "comment_content": comment_excerpt, - "app_url": app_url, - } - ) - return payloads - - @staticmethod - def _dispatch_mention_emails(payloads: Sequence[dict[str, str]]) -> None: - """Enqueue mention notification emails.""" - for payload in payloads: - send_workflow_comment_mention_email_task.delay(**payload) - - @staticmethod - def get_comments(tenant_id: str, app_id: str) -> Sequence[WorkflowComment]: - """Get all comments for a workflow.""" - with Session(db.engine) as session: - # Get all comments with eager loading - stmt = ( - select(WorkflowComment) - .options(selectinload(WorkflowComment.replies), selectinload(WorkflowComment.mentions)) - .where(WorkflowComment.tenant_id == tenant_id, WorkflowComment.app_id == app_id) - .order_by(desc(WorkflowComment.created_at)) - ) - - comments = session.scalars(stmt).all() - - # Batch preload all Account objects to avoid N+1 queries - WorkflowCommentService._preload_accounts(session, comments) - - return comments - - @staticmethod - def _preload_accounts(session: Session, comments: Sequence[WorkflowComment]) -> None: - """Batch preload Account objects for comments, replies, and mentions.""" - # Collect all user IDs - user_ids: set[str] = set() - for comment in comments: - user_ids.add(comment.created_by) - if comment.resolved_by: - user_ids.add(comment.resolved_by) - user_ids.update(reply.created_by for reply in comment.replies) - user_ids.update(mention.mentioned_user_id for mention in comment.mentions) - - if not user_ids: - return - - # Batch query all accounts - accounts = session.scalars(select(Account).where(Account.id.in_(user_ids))).all() - account_map = {str(account.id): account for account in accounts} - - # Cache accounts on objects - for comment in comments: - comment.cache_created_by_account(account_map.get(comment.created_by)) - comment.cache_resolved_by_account(account_map.get(comment.resolved_by) if comment.resolved_by else None) - for reply in comment.replies: - reply.cache_created_by_account(account_map.get(reply.created_by)) - for mention in comment.mentions: - mention.cache_mentioned_user_account(account_map.get(mention.mentioned_user_id)) - - @staticmethod - def get_comment(tenant_id: str, app_id: str, comment_id: str, session: Session | None = None) -> WorkflowComment: - """Get a specific comment.""" - - def _get_comment(session: Session) -> WorkflowComment: - stmt = ( - select(WorkflowComment) - .options(selectinload(WorkflowComment.replies), selectinload(WorkflowComment.mentions)) - .where( - WorkflowComment.id == comment_id, - WorkflowComment.tenant_id == tenant_id, - WorkflowComment.app_id == app_id, - ) - ) - comment = session.scalar(stmt) - - if not comment: - raise NotFound("Comment not found") - - # Preload accounts to avoid N+1 queries - WorkflowCommentService._preload_accounts(session, [comment]) - - return comment - - if session is not None: - return _get_comment(session) - else: - with Session(db.engine, expire_on_commit=False) as session: - return _get_comment(session) - - @staticmethod - def create_comment( - tenant_id: str, - app_id: str, - created_by: str, - content: str, - position_x: float, - position_y: float, - mentioned_user_ids: list[str] | None = None, - ) -> dict: - """Create a new workflow comment and send mention notification emails.""" - WorkflowCommentService._validate_content(content) - - with Session(db.engine) as session: - comment = WorkflowComment( - tenant_id=tenant_id, - app_id=app_id, - position_x=position_x, - position_y=position_y, - content=content, - created_by=created_by, - ) - - session.add(comment) - session.flush() # Get the comment ID for mentions - - # Create mentions if specified - mentioned_user_ids = WorkflowCommentService._filter_valid_mentioned_user_ids( - mentioned_user_ids or [], - session=session, - tenant_id=tenant_id, - ) - for user_id in mentioned_user_ids: - mention = WorkflowCommentMention( - comment_id=comment.id, - reply_id=None, # This is a comment mention, not reply mention - mentioned_user_id=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=created_by, - mentioned_user_ids=mentioned_user_ids, - content=content, - ) - - session.commit() - WorkflowCommentService._dispatch_mention_emails(mention_email_payloads) - - # Return only what we need - id and created_at - return {"id": comment.id, "created_at": comment.created_at} - - @staticmethod - def update_comment( - tenant_id: str, - app_id: str, - comment_id: str, - user_id: str, - content: str, - position_x: float | None = None, - position_y: float | None = None, - mentioned_user_ids: list[str] | None = None, - ) -> dict: - """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: - # Get comment with validation - stmt = select(WorkflowComment).where( - WorkflowComment.id == comment_id, - WorkflowComment.tenant_id == tenant_id, - WorkflowComment.app_id == app_id, - ) - comment = session.scalar(stmt) - - if not comment: - raise NotFound("Comment not found") - - # Only the creator can update the comment - if comment.created_by != user_id: - raise Forbidden("Only the comment creator can update it") - - # Update comment fields - comment.content = content - if position_x is not None: - comment.position_x = position_x - if position_y is not None: - comment.position_y = position_y - - 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) - - filtered_mentioned_user_ids = WorkflowCommentService._filter_valid_mentioned_user_ids( - mentioned_user_ids, - session=session, - tenant_id=tenant_id, - ) - 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, - ) - - session.commit() - WorkflowCommentService._dispatch_mention_emails(mention_email_payloads) - - return {"id": comment.id, "updated_at": comment.updated_at} - - @staticmethod - def delete_comment(tenant_id: str, app_id: str, comment_id: str, user_id: str) -> None: - """Delete a workflow comment.""" - with Session(db.engine, expire_on_commit=False) as session: - comment = WorkflowCommentService.get_comment(tenant_id, app_id, comment_id, session) - - # Only the creator can delete the comment - if comment.created_by != user_id: - raise Forbidden("Only the comment creator can delete it") - - # Delete associated mentions (both comment and reply mentions) - mentions = session.scalars( - select(WorkflowCommentMention).where(WorkflowCommentMention.comment_id == comment_id) - ).all() - for mention in mentions: - session.delete(mention) - - # Delete associated replies - replies = session.scalars( - select(WorkflowCommentReply).where(WorkflowCommentReply.comment_id == comment_id) - ).all() - for reply in replies: - session.delete(reply) - - session.delete(comment) - session.commit() - - @staticmethod - def resolve_comment(tenant_id: str, app_id: str, comment_id: str, user_id: str) -> WorkflowComment: - """Resolve a workflow comment.""" - with Session(db.engine, expire_on_commit=False) as session: - comment = WorkflowCommentService.get_comment(tenant_id, app_id, comment_id, session) - if comment.resolved: - return comment - - comment.resolved = True - comment.resolved_at = naive_utc_now() - comment.resolved_by = user_id - session.commit() - - return comment - - @staticmethod - def create_reply( - comment_id: str, content: str, created_by: str, mentioned_user_ids: list[str] | None = None - ) -> dict: - """Add a reply to a workflow comment and notify mentioned users.""" - WorkflowCommentService._validate_content(content) - - with Session(db.engine, expire_on_commit=False) as session: - # Check if comment exists - comment = session.get(WorkflowComment, comment_id) - if not comment: - raise NotFound("Comment not found") - - reply = WorkflowCommentReply(comment_id=comment_id, content=content, created_by=created_by) - - session.add(reply) - session.flush() # Get the reply ID for mentions - - # Create mentions if specified - mentioned_user_ids = WorkflowCommentService._filter_valid_mentioned_user_ids( - mentioned_user_ids or [], - session=session, - tenant_id=comment.tenant_id, - ) - for user_id in mentioned_user_ids: - # Create mention linking to specific reply - mention = WorkflowCommentMention(comment_id=comment_id, reply_id=reply.id, mentioned_user_id=user_id) - session.add(mention) - - mention_email_payloads = WorkflowCommentService._build_mention_email_payloads( - session=session, - tenant_id=comment.tenant_id, - app_id=comment.app_id, - mentioner_id=created_by, - mentioned_user_ids=mentioned_user_ids, - content=content, - ) - - session.commit() - WorkflowCommentService._dispatch_mention_emails(mention_email_payloads) - - return {"id": reply.id, "created_at": reply.created_at} - - @staticmethod - 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 = 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: - raise Forbidden("Only the reply creator can update it") - - reply.content = content - - # Update mentions - first remove existing mentions for this reply - existing_mentions = session.scalars( - select(WorkflowCommentMention).where(WorkflowCommentMention.reply_id == reply.id) - ).all() - existing_mentioned_user_ids = {mention.mentioned_user_id for mention in existing_mentions} - for mention in existing_mentions: - session.delete(mention) - - # Add mentions - raw_mentioned_user_ids = mentioned_user_ids or [] - comment = session.get(WorkflowComment, reply.comment_id) - mentioned_user_ids = [] - if comment: - mentioned_user_ids = WorkflowCommentService._filter_valid_mentioned_user_ids( - raw_mentioned_user_ids, - session=session, - tenant_id=comment.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=reply.comment_id, reply_id=reply.id, mentioned_user_id=user_id_str - ) - session.add(mention) - - mention_email_payloads: list[dict[str, str]] = [] - if comment: - mention_email_payloads = WorkflowCommentService._build_mention_email_payloads( - session=session, - tenant_id=comment.tenant_id, - app_id=comment.app_id, - mentioner_id=user_id, - mentioned_user_ids=new_mentioned_user_ids, - content=content, - ) - - session.commit() - session.refresh(reply) # Refresh to get updated timestamp - WorkflowCommentService._dispatch_mention_emails(mention_email_payloads) - - return {"id": reply.id, "updated_at": reply.updated_at} - - @staticmethod - 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 = 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: - raise Forbidden("Only the reply creator can delete it") - - # Delete associated mentions first - mentions = session.scalars( - select(WorkflowCommentMention).where(WorkflowCommentMention.reply_id == reply_id) - ).all() - for mention in mentions: - session.delete(mention) - - session.delete(reply) - session.commit() - - @staticmethod - def validate_comment_access(comment_id: str, tenant_id: str, app_id: str) -> WorkflowComment: - """Validate that a comment belongs to the specified tenant and app.""" - return WorkflowCommentService.get_comment(tenant_id, app_id, comment_id) diff --git a/api/tests/unit_tests/.ruff.toml b/api/tests/unit_tests/.ruff.toml index 3fd4cc35648..ea3bbf23001 100644 --- a/api/tests/unit_tests/.ruff.toml +++ b/api/tests/unit_tests/.ruff.toml @@ -381,7 +381,6 @@ extend-select = ["ANN401", "ARG"] "services/test_vector_service.py" = ["ARG001", "TID251"] "services/test_webhook_service_additional.py" = ["ANN401", "ARG002", "TID251"] "services/test_website_service.py" = ["TID251"] -"services/test_workflow_comment_service.py" = ["ARG001", "ARG002"] "services/test_workflow_run_service.py" = ["ANN401", "ARG002", "TID251"] "services/test_workflow_service.py" = ["ANN401", "ARG002", "TID251"] "services/tools/test_builtin_tools_manage_service.py" = ["ARG001", "ARG002"] 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 337a4f31881..4712ee48294 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 @@ -1,25 +1,59 @@ +"""Console workflow comment routes keep their admission policy and wire contracts on the application service.""" + from __future__ import annotations -from contextlib import nullcontext +from collections.abc import Callable +from contextlib import AbstractContextManager, nullcontext from dataclasses import dataclass from datetime import datetime +from inspect import unwrap +from types import SimpleNamespace from unittest.mock import MagicMock, PropertyMock, patch +from uuid import UUID import pytest from flask import Flask -from werkzeug.exceptions import Forbidden +from werkzeug.exceptions import Forbidden, HTTPException -from controllers.console import console_ns -from controllers.console import wraps as console_wraps +from controllers.common.errors import ForbiddenError, InvalidArgumentError, NotFoundError +from controllers.console import console_ns, flask_admission from controllers.console.app import workflow_comment as workflow_comment_module -from controllers.console.app import wraps as app_wraps -from enums import DeploymentEdition -from libs import login as login_lib -from models import App, Tenant, WorkflowComment, WorkflowCommentMention, WorkflowCommentReply -from models.account import Account, AccountStatus, TenantAccountRole -from models.model import AppMode, IconType +from controllers.console.app.error import AppNotFoundError +from controllers.console.app.workflow_comment import ( + WorkflowCommentDetailApi, + WorkflowCommentListApi, + WorkflowCommentMentionUsersApi, + WorkflowCommentReplyApi, + WorkflowCommentReplyDetailApi, + WorkflowCommentResolveApi, +) +from libs.login import AccountWithTenant +from machinery.context import RequestContext +from models.account import TenantAccountRole +from services.app.workflow_comment_service import ( + InvalidMentionedUserIdError, + InvalidWorkflowCommentContentError, + WorkflowCommentAccount, + WorkflowCommentAppNotFoundError, + WorkflowCommentCreated, + WorkflowCommentDetail, + WorkflowCommentDraft, + WorkflowCommentEdit, + WorkflowCommentMentionRecord, + WorkflowCommentNotFoundError, + WorkflowCommentPermissionError, + WorkflowCommentReplyDraft, + WorkflowCommentReplyNotFoundError, + WorkflowCommentReplyRecord, + WorkflowCommentResolved, + WorkflowCommentSummary, + WorkflowCommentUpdated, +) from tests.unit_tests.config_override import apply_config_overrides +from tests.unit_tests.model_factories import make_account +WORKSPACE_ID = "tenant-123" +APP_ID = UUID("11111111-1111-1111-1111-111111111111") JAN_1_2024_NOON = datetime(2024, 1, 1, 12, 0, 0) JAN_1_2024_NOON_TS = int(JAN_1_2024_NOON.timestamp()) JAN_1_2024_1201 = datetime(2024, 1, 1, 12, 1, 0) @@ -29,266 +63,275 @@ JAN_1_2024_1202_TS = int(JAN_1_2024_1202.timestamp()) JAN_1_2024_1203 = datetime(2024, 1, 1, 12, 3, 0) JAN_1_2024_1203_TS = int(JAN_1_2024_1203.timestamp()) - -def _make_account(role: TenantAccountRole) -> Account: - account = Account(name="tester", email="tester@example.com") - account.status = AccountStatus.ACTIVE - account.role = role - account.id = "account-123" # type: ignore[assignment] - tenant = Tenant(name="Test tenant") - tenant.id = "tenant-123" - account._current_tenant = tenant - account._get_current_object = lambda: account # type: ignore[attr-defined] - return account +AUTHOR = WorkflowCommentAccount( + id="account-123", name="tester", email="tester@example.com", avatar="https://example.com/avatar.png" +) +AUTHOR_JSON = { + "id": "account-123", + "name": "tester", + "email": "tester@example.com", + "avatar_url": "https://example.com/avatar.png", +} +MENTIONED = WorkflowCommentAccount(id="account-456", name="mentioned", email="mentioned@example.com", avatar=None) +MENTIONED_JSON = {"id": "account-456", "name": "mentioned", "email": "mentioned@example.com", "avatar_url": None} -def _make_app() -> App: - return App( - id="app-123", - tenant_id="tenant-123", - name="Workflow comments app", - description="", - mode=AppMode.WORKFLOW, - icon_type=IconType.EMOJI, - icon="robot", - icon_background="#FFFFFF", - enable_site=True, - enable_api=True, - max_active_requests=None, +def admission(method: Callable[..., object]) -> Callable[..., object]: + """Skip setup/login wrappers and enter at the admission adapter that builds the RequestContext.""" + return unwrap(method, stop=lambda view: "inject_request_context" in view.__code__.co_qualname) + + +@pytest.fixture +def comments(monkeypatch: pytest.MonkeyPatch) -> MagicMock: + service = MagicMock() + members = MagicMock() + monkeypatch.setattr( + workflow_comment_module, + "application_services", + lambda: SimpleNamespace(workflow_comments=service, workspaces=SimpleNamespace(member_queries=members)), ) + service.members = members + return service -def _patch_console_guards(monkeypatch: pytest.MonkeyPatch, account: Account, app_model: App) -> None: - apply_config_overrides(monkeypatch, LOGIN_DISABLED=True, DEPLOYMENT_EDITION=DeploymentEdition.CLOUD) - monkeypatch.setattr(login_lib, "current_user", account) - monkeypatch.setattr(login_lib, "current_account_with_tenant", lambda: (account, account.current_tenant_id)) - monkeypatch.setattr(login_lib, "check_csrf_token", lambda *_, **__: None) - monkeypatch.setattr(console_wraps, "current_account_with_tenant", lambda: (account, account.current_tenant_id)) - monkeypatch.setattr(app_wraps, "current_account_with_tenant", lambda: (account, account.current_tenant_id)) - monkeypatch.setattr(app_wraps, "_load_app_model_from_scoped_session", lambda _app_id: app_model) +def _admit(monkeypatch: pytest.MonkeyPatch, role: TenantAccountRole, *, rbac_enabled: bool = False) -> str: + account = make_account(account_id="account-123", role=role) + apply_config_overrides(monkeypatch, RBAC_ENABLED=rbac_enabled) + monkeypatch.setattr( + flask_admission, "current_account_with_tenant", lambda: AccountWithTenant(account, WORKSPACE_ID) + ) + monkeypatch.setattr(flask_admission, "get_trace_id", lambda: None) + return account.id -def _patch_write_services(monkeypatch: pytest.MonkeyPatch) -> None: - for method_name in ( - "create_comment", - "update_comment", - "delete_comment", - "resolve_comment", - "validate_comment_access", - "create_reply", - "update_reply", - "delete_reply", - ): - monkeypatch.setattr(workflow_comment_module.WorkflowCommentService, method_name, MagicMock()) - - -def _patch_payload(payload: dict[str, object] | None): +def _payload(payload: dict[str, object] | None) -> AbstractContextManager[object]: if payload is None: return nullcontext() - return patch.object( - type(console_ns), - "payload", - new_callable=PropertyMock, - return_value=payload, - ) + return patch.object(type(console_ns), "payload", new_callable=PropertyMock, return_value=payload) @dataclass(frozen=True) class WriteCase: - resource_cls: type - method_name: str - path: str - kwargs: dict[str, str] + method: Callable[..., object] + http_method: str + kwargs: dict[str, object] + service: Callable[[MagicMock], MagicMock] + service_return: object = None payload: dict[str, object] | None = None -@dataclass(frozen=True) -class MutationResponseCase: - resource_cls: type - method_name: str - path: str - kwargs: dict[str, str] - service_method_name: str - service_return: object - expected_response: dict[str, object] - payload: dict[str, object] | None = None - expected_status: int | None = None - - -def unwrap_response(result: object) -> tuple[dict[str, object], int | None]: - if isinstance(result, tuple): - response, status = result - assert isinstance(response, dict) - assert isinstance(status, int) - return response, status - - assert isinstance(result, dict) - return result, None - - -@pytest.mark.parametrize( - "case", - [ - WriteCase( - resource_cls=workflow_comment_module.WorkflowCommentListApi, - method_name="post", - path="/console/api/apps/app-123/workflow/comments", - kwargs={"app_id": "app-123"}, - payload={"content": "hello", "position_x": 1.0, "position_y": 2.0, "mentioned_user_ids": []}, - ), - WriteCase( - resource_cls=workflow_comment_module.WorkflowCommentDetailApi, - method_name="put", - path="/console/api/apps/app-123/workflow/comments/comment-1", - kwargs={"app_id": "app-123", "comment_id": "comment-1"}, - payload={"content": "hello", "position_x": 1.0, "position_y": 2.0, "mentioned_user_ids": []}, - ), - WriteCase( - resource_cls=workflow_comment_module.WorkflowCommentDetailApi, - method_name="delete", - path="/console/api/apps/app-123/workflow/comments/comment-1", - kwargs={"app_id": "app-123", "comment_id": "comment-1"}, - ), - WriteCase( - resource_cls=workflow_comment_module.WorkflowCommentResolveApi, - method_name="post", - path="/console/api/apps/app-123/workflow/comments/comment-1/resolve", - kwargs={"app_id": "app-123", "comment_id": "comment-1"}, - ), - WriteCase( - resource_cls=workflow_comment_module.WorkflowCommentReplyApi, - method_name="post", - path="/console/api/apps/app-123/workflow/comments/comment-1/replies", - kwargs={"app_id": "app-123", "comment_id": "comment-1"}, - payload={"content": "reply", "mentioned_user_ids": []}, - ), - WriteCase( - resource_cls=workflow_comment_module.WorkflowCommentReplyDetailApi, - method_name="put", - path="/console/api/apps/app-123/workflow/comments/comment-1/replies/reply-1", - kwargs={"app_id": "app-123", "comment_id": "comment-1", "reply_id": "reply-1"}, - payload={"content": "reply", "mentioned_user_ids": []}, - ), - WriteCase( - resource_cls=workflow_comment_module.WorkflowCommentReplyDetailApi, - method_name="delete", - path="/console/api/apps/app-123/workflow/comments/comment-1/replies/reply-1", - kwargs={"app_id": "app-123", "comment_id": "comment-1", "reply_id": "reply-1"}, - ), - ], +CREATE_COMMENT = WriteCase( + WorkflowCommentListApi.post, + "POST", + {"app_id": APP_ID}, + lambda comments: comments.create_comment, + WorkflowCommentCreated(id="comment-1", created_at=JAN_1_2024_NOON), + {"content": "hello", "position_x": 1.0, "position_y": 2.0, "mentioned_user_ids": []}, ) -def test_write_endpoints_require_edit_permission(app: Flask, monkeypatch: pytest.MonkeyPatch, case: WriteCase) -> None: - app.config.setdefault("RESTX_MASK_HEADER", "X-Fields") - account = _make_account(TenantAccountRole.NORMAL) - app_model = _make_app() - _patch_console_guards(monkeypatch, account, app_model) - _patch_write_services(monkeypatch) - - with app.test_request_context(case.path, method=case.method_name.upper(), json=case.payload): - with _patch_payload(case.payload): - handler = getattr(case.resource_cls(), case.method_name) - with pytest.raises(Forbidden): - handler(**case.kwargs) +UPDATE_COMMENT = WriteCase( + WorkflowCommentDetailApi.put, + "PUT", + {"app_id": APP_ID, "comment_id": "comment-1"}, + lambda comments: comments.update_comment, + WorkflowCommentUpdated(id="comment-1", updated_at=JAN_1_2024_NOON), + {"content": "hello"}, +) +DELETE_COMMENT = WriteCase( + WorkflowCommentDetailApi.delete, + "DELETE", + {"app_id": APP_ID, "comment_id": "comment-1"}, + lambda comments: comments.delete_comment, +) +RESOLVE_COMMENT = WriteCase( + WorkflowCommentResolveApi.post, + "POST", + {"app_id": APP_ID, "comment_id": "comment-1"}, + lambda comments: comments.resolve_comment, + WorkflowCommentResolved(id="comment-1", resolved=True, resolved_at=JAN_1_2024_NOON, resolved_by="account-123"), +) +CREATE_REPLY = WriteCase( + WorkflowCommentReplyApi.post, + "POST", + {"app_id": APP_ID, "comment_id": "comment-1"}, + lambda comments: comments.create_reply, + WorkflowCommentCreated(id="reply-1", created_at=JAN_1_2024_NOON), + {"content": "reply", "mentioned_user_ids": []}, +) +UPDATE_REPLY = WriteCase( + WorkflowCommentReplyDetailApi.put, + "PUT", + {"app_id": APP_ID, "comment_id": "comment-1", "reply_id": "reply-1"}, + lambda comments: comments.update_reply, + WorkflowCommentUpdated(id="reply-1", updated_at=JAN_1_2024_NOON), + {"content": "reply", "mentioned_user_ids": []}, +) +DELETE_REPLY = WriteCase( + WorkflowCommentReplyDetailApi.delete, + "DELETE", + {"app_id": APP_ID, "comment_id": "comment-1", "reply_id": "reply-1"}, + lambda comments: comments.delete_reply, +) +WRITE_CASES = [ + pytest.param(CREATE_COMMENT, id="create-comment"), + pytest.param(UPDATE_COMMENT, id="update-comment"), + pytest.param(DELETE_COMMENT, id="delete-comment"), + pytest.param(RESOLVE_COMMENT, id="resolve-comment"), + pytest.param(CREATE_REPLY, id="create-reply"), + pytest.param(UPDATE_REPLY, id="update-reply"), + pytest.param(DELETE_REPLY, id="delete-reply"), +] -def test_create_comment_allows_editor(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) +def _call(app: Flask, case: WriteCase) -> object: + with app.test_request_context(method=case.http_method, json=case.payload), _payload(case.payload): + return admission(case.method)(None, **case.kwargs) - create_comment_mock = MagicMock(return_value={"id": "comment-1"}) - monkeypatch.setattr(workflow_comment_module.WorkflowCommentService, "create_comment", create_comment_mock) + +@pytest.mark.parametrize("case", WRITE_CASES) +@pytest.mark.parametrize("role", [TenantAccountRole.NORMAL, TenantAccountRole.DATASET_OPERATOR]) +def test_write_endpoints_reject_read_only_roles( + app: Flask, monkeypatch: pytest.MonkeyPatch, comments: MagicMock, case: WriteCase, role: TenantAccountRole +) -> None: + _admit(monkeypatch, role) + + with pytest.raises(Forbidden): + _call(app, case) + + case.service(comments).assert_not_called() + + +@pytest.mark.parametrize("case", WRITE_CASES) +@pytest.mark.parametrize("role", [TenantAccountRole.OWNER, TenantAccountRole.ADMIN, TenantAccountRole.EDITOR]) +def test_write_endpoints_admit_edit_roles( + app: Flask, monkeypatch: pytest.MonkeyPatch, comments: MagicMock, case: WriteCase, role: TenantAccountRole +) -> None: + _admit(monkeypatch, role) + case.service(comments).return_value = case.service_return + + _call(app, case) + + case.service(comments).assert_called_once() + + +@pytest.mark.parametrize("case", WRITE_CASES) +def test_rbac_replaces_the_legacy_role_gate( + app: Flask, monkeypatch: pytest.MonkeyPatch, comments: MagicMock, case: WriteCase +) -> None: + # Like `edit_permission_required`, the role gate is skipped under RBAC; these routes declare no RBAC checks. + _admit(monkeypatch, TenantAccountRole.NORMAL, rbac_enabled=True) + case.service(comments).return_value = case.service_return + + _call(app, case) + + case.service(comments).assert_called_once() + + +def test_create_comment_passes_request_context_and_typed_draft( + app: Flask, monkeypatch: pytest.MonkeyPatch, comments: MagicMock +) -> None: + account_id = _admit(monkeypatch, TenantAccountRole.EDITOR) + comments.create_comment.return_value = CREATE_COMMENT.service_return payload: dict[str, object] = { "content": "hello", "position_x": 1.0, "position_y": 2.0, - "mentioned_user_ids": [], + "mentioned_user_ids": ["user-1"], } - with app.test_request_context("/console/api/apps/app-123/workflow/comments", method="POST", json=payload): - with _patch_payload(payload): - result = workflow_comment_module.WorkflowCommentListApi().post(app_id="app-123") + with app.test_request_context(method="POST", json=payload), _payload(payload): + result = admission(WorkflowCommentListApi.post)(None, app_id=APP_ID) - response, status = unwrap_response(result) - assert response["id"] == "comment-1" - assert status == 201 - create_comment_mock.assert_called_once_with( - tenant_id="tenant-123", - app_id="app-123", - created_by="account-123", - content="hello", - position_x=1.0, - position_y=2.0, - mentioned_user_ids=[], + assert result == ({"id": "comment-1", "created_at": JAN_1_2024_NOON_TS}, 201) + context, app_id, draft = comments.create_comment.call_args.args + assert isinstance(context, RequestContext) + assert (context.account_id, context.active_workspace_id) == (account_id, WORKSPACE_ID) + assert app_id == str(APP_ID) + assert draft == WorkflowCommentDraft( + content="hello", position_x=1.0, position_y=2.0, mentioned_user_ids=("user-1",) ) -def test_update_comment_omits_mentions_when_payload_does_not_include_them( - app: Flask, monkeypatch: pytest.MonkeyPatch +def test_update_comment_keeps_omitted_mentions_omitted( + app: Flask, monkeypatch: pytest.MonkeyPatch, comments: MagicMock ) -> 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": JAN_1_2024_NOON}) - monkeypatch.setattr(workflow_comment_module.WorkflowCommentService, "update_comment", update_comment_mock) + _admit(monkeypatch, TenantAccountRole.EDITOR) + comments.update_comment.return_value = UPDATE_COMMENT.service_return payload: dict[str, object] = {"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): - result = workflow_comment_module.WorkflowCommentDetailApi().put(app_id="app-123", comment_id="comment-1") + with app.test_request_context(method="PUT", json=payload), _payload(payload): + result = admission(WorkflowCommentDetailApi.put)(None, app_id=APP_ID, comment_id="comment-1") - response, status = unwrap_response(result) - assert response == {"id": "comment-1", "updated_at": JAN_1_2024_NOON_TS} - assert status is None - 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, + assert result == {"id": "comment-1", "updated_at": JAN_1_2024_NOON_TS} + _, _, comment_id, edit = comments.update_comment.call_args.args + assert comment_id == "comment-1" + assert edit == WorkflowCommentEdit(content="hello", position_x=10.0, position_y=20.0, mentioned_user_ids=None) + + +def test_reply_update_passes_typed_draft(app: Flask, monkeypatch: pytest.MonkeyPatch, comments: MagicMock) -> None: + _admit(monkeypatch, TenantAccountRole.EDITOR) + comments.update_reply.return_value = UPDATE_REPLY.service_return + payload: dict[str, object] = {"content": "reply", "mentioned_user_ids": ["user-1"]} + + with app.test_request_context(method="PUT", json=payload), _payload(payload): + result = admission(WorkflowCommentReplyDetailApi.put)( + None, app_id=APP_ID, comment_id="comment-1", reply_id="reply-1" + ) + + assert result == {"id": "reply-1", "updated_at": JAN_1_2024_NOON_TS} + _, _, comment_id, reply_id, draft = comments.update_reply.call_args.args + assert (comment_id, reply_id) == ("comment-1", "reply-1") + assert draft == WorkflowCommentReplyDraft(content="reply", mentioned_user_ids=("user-1",)) + + +@pytest.mark.parametrize( + ("case", "expected"), + [ + pytest.param( + RESOLVE_COMMENT, + {"id": "comment-1", "resolved": True, "resolved_at": JAN_1_2024_NOON_TS, "resolved_by": "account-123"}, + id="resolve", + ), + pytest.param(CREATE_REPLY, ({"id": "reply-1", "created_at": JAN_1_2024_NOON_TS}, 201), id="create-reply"), + pytest.param(DELETE_COMMENT, ("", 204), id="delete-comment"), + pytest.param(DELETE_REPLY, ("", 204), id="delete-reply"), + ], +) +def test_mutation_endpoints_serialize_responses( + app: Flask, monkeypatch: pytest.MonkeyPatch, comments: MagicMock, case: WriteCase, expected: object +) -> None: + _admit(monkeypatch, TenantAccountRole.EDITOR) + case.service(comments).return_value = case.service_return + + assert _call(app, case) == expected + + +@pytest.mark.parametrize("role", [TenantAccountRole.NORMAL, TenantAccountRole.DATASET_OPERATOR]) +def test_list_comments_serializes_summaries_for_any_member( + app: Flask, monkeypatch: pytest.MonkeyPatch, comments: MagicMock, role: TenantAccountRole +) -> None: + _admit(monkeypatch, role) + comments.list_comments.return_value = ( + WorkflowCommentSummary( + id="comment-1", + position_x=1.5, + position_y=2.5, + content="hello", + created_by="account-123", + created_by_account=AUTHOR, + created_at=datetime.fromtimestamp(1_700_000_000), + updated_at=datetime.fromtimestamp(1_700_000_001), + resolved=False, + resolved_at=None, + resolved_by=None, + resolved_by_account=None, + reply_count=0, + mention_count=0, + participants=(AUTHOR,), + ), ) - -def test_list_comments_serializes_response_model(app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: - app.config.setdefault("RESTX_MASK_HEADER", "X-Fields") - account = _make_account(TenantAccountRole.NORMAL) - app_model = _make_app() - _patch_console_guards(monkeypatch, account, app_model) - - comment_author = Account( - name="tester", - email="tester@example.com", - avatar="https://example.com/avatar.png", - status=AccountStatus.ACTIVE, - ) - comment_author.id = "account-123" - comment = WorkflowComment( - tenant_id="tenant-123", - app_id="app-123", - position_x=1.5, - position_y=2.5, - content="hello", - created_by="account-123", - resolved=False, - resolved_at=None, - resolved_by=None, - ) - comment.id = "comment-1" - comment.created_at = datetime.fromtimestamp(1_700_000_000) - comment.updated_at = datetime.fromtimestamp(1_700_000_001) - comment.cache_created_by_account(comment_author) - comment.cache_resolved_by_account(None) - get_comments_mock = MagicMock(return_value=[comment]) - monkeypatch.setattr(workflow_comment_module.WorkflowCommentService, "get_comments", get_comments_mock) - - with app.test_request_context("/console/api/apps/app-123/workflow/comments", method="GET"): - response = workflow_comment_module.WorkflowCommentListApi().get(app_id="app-123") + with app.test_request_context(): + response = admission(WorkflowCommentListApi.get)(None, app_id=APP_ID) assert response == { "data": [ @@ -298,12 +341,7 @@ def test_list_comments_serializes_response_model(app: Flask, monkeypatch: pytest "position_y": 2.5, "content": "hello", "created_by": "account-123", - "created_by_account": { - "id": "account-123", - "name": "tester", - "email": "tester@example.com", - "avatar_url": "https://example.com/avatar.png", - }, + "created_by_account": AUTHOR_JSON, "created_at": 1_700_000_000, "updated_at": 1_700_000_001, "resolved": False, @@ -312,74 +350,45 @@ def test_list_comments_serializes_response_model(app: Flask, monkeypatch: pytest "resolved_by_account": None, "reply_count": 0, "mention_count": 0, - "participants": [ - { - "id": "account-123", - "name": "tester", - "email": "tester@example.com", - "avatar_url": "https://example.com/avatar.png", - } - ], + "participants": [AUTHOR_JSON], } ] } - get_comments_mock.assert_called_once_with(tenant_id="tenant-123", app_id="app-123") -def test_get_comment_serializes_detail_response_model(app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: - app.config.setdefault("RESTX_MASK_HEADER", "X-Fields") - account = _make_account(TenantAccountRole.NORMAL) - app_model = _make_app() - _patch_console_guards(monkeypatch, account, app_model) - - comment_author = Account( - name="tester", - email="tester@example.com", - avatar="https://example.com/avatar.png", - status=AccountStatus.ACTIVE, - ) - comment_author.id = "account-123" - mentioned_user = Account( - name="mentioned", - email="mentioned@example.com", - avatar=None, - status=AccountStatus.ACTIVE, - ) - mentioned_user.id = "account-456" - comment = WorkflowComment( - tenant_id="tenant-123", - app_id="app-123", +def test_get_comment_serializes_detail(app: Flask, monkeypatch: pytest.MonkeyPatch, comments: MagicMock) -> None: + _admit(monkeypatch, TenantAccountRole.NORMAL) + comments.get_comment.return_value = WorkflowCommentDetail( + id="comment-1", position_x=1.5, position_y=2.5, content="hello", created_by="account-123", + created_by_account=AUTHOR, + created_at=JAN_1_2024_NOON, + updated_at=JAN_1_2024_1201, resolved=True, resolved_at=JAN_1_2024_1202, resolved_by="account-123", + resolved_by_account=AUTHOR, + replies=( + WorkflowCommentReplyRecord( + id="reply-1", + content="reply", + created_by="account-456", + created_by_account=MENTIONED, + created_at=JAN_1_2024_1203, + ), + ), + mentions=( + WorkflowCommentMentionRecord( + mentioned_user_id="account-456", mentioned_user_account=MENTIONED, reply_id="reply-1" + ), + ), ) - comment.id = "comment-1" - comment.created_at = JAN_1_2024_NOON - comment.updated_at = JAN_1_2024_1201 - comment.cache_created_by_account(comment_author) - comment.cache_resolved_by_account(comment_author) - reply = WorkflowCommentReply(comment_id=comment.id, content="reply", created_by="account-456") - reply.id = "reply-1" - reply.created_at = JAN_1_2024_1203 - reply.updated_at = JAN_1_2024_1203 - reply.cache_created_by_account(mentioned_user) - mention = WorkflowCommentMention( - comment_id=comment.id, - mentioned_user_id="account-456", - reply_id=reply.id, - ) - mention.cache_mentioned_user_account(mentioned_user) - comment.replies.append(reply) - comment.mentions.append(mention) - get_comment_mock = MagicMock(return_value=comment) - monkeypatch.setattr(workflow_comment_module.WorkflowCommentService, "get_comment", get_comment_mock) - with app.test_request_context("/console/api/apps/app-123/workflow/comments/comment-1", method="GET"): - response = workflow_comment_module.WorkflowCommentDetailApi().get(app_id="app-123", comment_id="comment-1") + with app.test_request_context(): + response = admission(WorkflowCommentDetailApi.get)(None, app_id=APP_ID, comment_id="comment-1") assert response == { "id": "comment-1", @@ -387,121 +396,115 @@ def test_get_comment_serializes_detail_response_model(app: Flask, monkeypatch: p "position_y": 2.5, "content": "hello", "created_by": "account-123", - "created_by_account": { - "id": "account-123", - "name": "tester", - "email": "tester@example.com", - "avatar_url": "https://example.com/avatar.png", - }, + "created_by_account": AUTHOR_JSON, "created_at": JAN_1_2024_NOON_TS, "updated_at": JAN_1_2024_1201_TS, "resolved": True, "resolved_at": JAN_1_2024_1202_TS, "resolved_by": "account-123", - "resolved_by_account": { - "id": "account-123", - "name": "tester", - "email": "tester@example.com", - "avatar_url": "https://example.com/avatar.png", - }, + "resolved_by_account": AUTHOR_JSON, "replies": [ { "id": "reply-1", "content": "reply", "created_by": "account-456", - "created_by_account": { - "id": "account-456", - "name": "mentioned", - "email": "mentioned@example.com", - "avatar_url": None, - }, + "created_by_account": MENTIONED_JSON, "created_at": JAN_1_2024_1203_TS, } ], "mentions": [ - { - "mentioned_user_id": "account-456", - "mentioned_user_account": { - "id": "account-456", - "name": "mentioned", - "email": "mentioned@example.com", - "avatar_url": None, - }, - "reply_id": "reply-1", - } + {"mentioned_user_id": "account-456", "mentioned_user_account": MENTIONED_JSON, "reply_id": "reply-1"} ], } - get_comment_mock.assert_called_once_with(tenant_id="tenant-123", app_id="app-123", comment_id="comment-1") + _, app_id, comment_id = comments.get_comment.call_args.args + assert (app_id, comment_id) == (str(APP_ID), "comment-1") + + +def test_mention_users_requires_the_app_and_lists_workspace_members( + app: Flask, monkeypatch: pytest.MonkeyPatch, comments: MagicMock +) -> None: + _admit(monkeypatch, TenantAccountRole.NORMAL) + comments.members.list_members.return_value = [ + SimpleNamespace( + id="account-456", + name="mentioned", + email="mentioned@example.com", + avatar=None, + last_login_at=None, + last_active_at=None, + created_at=JAN_1_2024_NOON, + role="normal", + status="active", + ) + ] + + with app.test_request_context(): + result = admission(WorkflowCommentMentionUsersApi.get)(None, app_id=APP_ID) + + assert isinstance(result, tuple) + response, status = result + assert isinstance(response, dict) + assert status == 200 + assert [user["id"] for user in response["users"]] == ["account-456"] + comments.ensure_app.assert_called_once() + assert comments.ensure_app.call_args.args[1] == str(APP_ID) + comments.members.list_members.assert_called_once_with(WORKSPACE_ID) + + +def test_mention_users_rejects_unaddressable_app( + app: Flask, monkeypatch: pytest.MonkeyPatch, comments: MagicMock +) -> None: + _admit(monkeypatch, TenantAccountRole.NORMAL) + comments.ensure_app.side_effect = WorkflowCommentAppNotFoundError + + with app.test_request_context(), pytest.raises(AppNotFoundError): + admission(WorkflowCommentMentionUsersApi.get)(None, app_id=APP_ID) + + comments.members.list_members.assert_not_called() @pytest.mark.parametrize( - "case", + ("error", "expected_type", "expected_message"), [ - MutationResponseCase( - resource_cls=workflow_comment_module.WorkflowCommentResolveApi, - method_name="post", - path="/console/api/apps/app-123/workflow/comments/comment-1/resolve", - kwargs={"app_id": "app-123", "comment_id": "comment-1"}, - service_method_name="resolve_comment", - service_return={ - "id": "comment-1", - "resolved": True, - "resolved_at": JAN_1_2024_NOON, - "resolved_by": "account-123", - }, - expected_response={ - "id": "comment-1", - "resolved": True, - "resolved_at": JAN_1_2024_NOON_TS, - "resolved_by": "account-123", - }, + pytest.param(WorkflowCommentAppNotFoundError(), AppNotFoundError, "App not found.", id="app"), + pytest.param(WorkflowCommentNotFoundError(), NotFoundError, "Comment not found", id="comment"), + pytest.param(WorkflowCommentReplyNotFoundError(), NotFoundError, "Reply not found", id="reply"), + pytest.param( + WorkflowCommentPermissionError("Only the comment creator can update it"), + ForbiddenError, + "Only the comment creator can update it", + id="permission", ), - MutationResponseCase( - resource_cls=workflow_comment_module.WorkflowCommentReplyApi, - method_name="post", - path="/console/api/apps/app-123/workflow/comments/comment-1/replies", - kwargs={"app_id": "app-123", "comment_id": "comment-1"}, - payload={"content": "reply", "mentioned_user_ids": []}, - service_method_name="create_reply", - service_return={"id": "reply-1", "created_at": JAN_1_2024_NOON}, - expected_response={"id": "reply-1", "created_at": JAN_1_2024_NOON_TS}, - expected_status=201, + pytest.param( + InvalidWorkflowCommentContentError("Comment content cannot be empty"), + InvalidArgumentError, + "Comment content cannot be empty", + id="content", ), - MutationResponseCase( - resource_cls=workflow_comment_module.WorkflowCommentReplyDetailApi, - method_name="put", - path="/console/api/apps/app-123/workflow/comments/comment-1/replies/reply-1", - kwargs={"app_id": "app-123", "comment_id": "comment-1", "reply_id": "reply-1"}, - payload={"content": "reply", "mentioned_user_ids": []}, - service_method_name="update_reply", - service_return={"id": "reply-1", "updated_at": JAN_1_2024_NOON}, - expected_response={"id": "reply-1", "updated_at": JAN_1_2024_NOON_TS}, + pytest.param( + InvalidMentionedUserIdError("bad"), InvalidArgumentError, "bad is not a valid uuid.", id="mention-id" ), ], ) -def test_mutation_endpoints_serialize_response_models( - app: Flask, monkeypatch: pytest.MonkeyPatch, case: MutationResponseCase +def test_domain_errors_map_to_existing_transport_errors( + app: Flask, + monkeypatch: pytest.MonkeyPatch, + comments: MagicMock, + error: Exception, + expected_type: type[Exception], + expected_message: str, ) -> 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) - _patch_write_services(monkeypatch) - monkeypatch.setattr( - workflow_comment_module.WorkflowCommentService, - case.service_method_name, - MagicMock(return_value=case.service_return), - ) + _admit(monkeypatch, TenantAccountRole.EDITOR) + comments.update_comment.side_effect = error - with app.test_request_context(case.path, method=case.method_name.upper(), json=case.payload): - with _patch_payload(case.payload): - result = getattr(case.resource_cls(), case.method_name)(**case.kwargs) + with pytest.raises(expected_type) as raised: + _call(app, UPDATE_COMMENT) - response, status = unwrap_response(result) - assert response == case.expected_response - assert status == case.expected_status + exception = raised.value + message = exception.description if isinstance(exception, HTTPException) else str(exception) + assert message == expected_message def test_workflow_comment_response_schemas_are_registered() -> None: - assert workflow_comment_module.WorkflowCommentBasicList.__name__ in workflow_comment_module.console_ns.models - assert workflow_comment_module.WorkflowCommentDetail.__name__ in workflow_comment_module.console_ns.models + assert workflow_comment_module.WorkflowCommentBasicList.__name__ in console_ns.models + assert workflow_comment_module.WorkflowCommentDetail.__name__ in console_ns.models diff --git a/api/tests/unit_tests/extensions/test_ext_application_services.py b/api/tests/unit_tests/extensions/test_ext_application_services.py index 4b20278d464..8b41d60da44 100644 --- a/api/tests/unit_tests/extensions/test_ext_application_services.py +++ b/api/tests/unit_tests/extensions/test_ext_application_services.py @@ -47,6 +47,7 @@ from repositories.account_activation_repository import SQLAlchemyAccountActivati from repositories.account_integration_repository import SQLAlchemyAccountIntegrationRepository from repositories.app.site_command_repository import AppSiteCommandRepository from repositories.app.tracing_config_repository import SQLAlchemyAppTracingConfigRepository +from repositories.app.workflow_comment_repository import WorkflowCommentRepository from repositories.app_scoped_end_user_repository import AppScopedEndUserRepo from repositories.app_statistic_query_repository import AppStatisticQueryRepository from repositories.human_input_file_upload_repository import SQLAlchemyHumanInputFileUploadRepository @@ -93,6 +94,8 @@ from services.api_based_extension_application_service import APIBasedExtensionAp from services.app.api_key_service import AppApiKeyService from services.app.console_gateway import EnterpriseConsoleAppAccess from services.app.creators_platform_gateway import CreatorsPlatformGateway +from services.app.workflow_comment_mention_gateway import CeleryWorkflowCommentMentionNotifier +from services.app.workflow_comment_service import WorkflowCommentService from services.app_generate_service import AppGenerateService from services.app_preview_query_service import AppPreviewRef, AppPreviewUnavailableError from services.app_scoped_end_user_query_service import AppScopedEndUserQueryService @@ -600,6 +603,22 @@ def test_build_application_services_wires_workflow_app_log_boundary( assert services.workflow_app_logs._logs._session_factory is sqlite_session_factory +def test_build_application_services_wires_workflow_comment_boundary( + sqlite_session_factory: sessionmaker[Session], +) -> None: + services = ext_application_services.build_application_services( + database_client=sqlite_session_factory, + deployment_edition=DeploymentEdition.COMMUNITY, + initialization_password="", + redis=_redis(), + ) + + assert isinstance(services.workflow_comments, WorkflowCommentService) + assert isinstance(services.workflow_comments._comments, WorkflowCommentRepository) + assert services.workflow_comments._comments._session_factory is sqlite_session_factory + assert isinstance(services.workflow_comments._notifier, CeleryWorkflowCommentMentionNotifier) + + def test_build_application_services_wires_app_statistic_boundary( sqlite_session_factory: sessionmaker[Session], ) -> None: diff --git a/api/tests/unit_tests/models/test_comment_models.py b/api/tests/unit_tests/models/test_comment_models.py deleted file mode 100644 index 0dc1e3339d2..00000000000 --- a/api/tests/unit_tests/models/test_comment_models.py +++ /dev/null @@ -1,131 +0,0 @@ -from uuid import uuid4 - -import pytest -from sqlalchemy.orm import Session - -from models.account import Account -from models.comment import WorkflowComment, WorkflowCommentMention, WorkflowCommentReply -from tests.unit_tests.model_factories import make_account - - -def _account(name: str) -> Account: - return make_account(account_id=str(uuid4()), name=name, email=f"{name.lower()}@example.com") - - -def _comment(created_by: str, resolved_by: str | None = None) -> WorkflowComment: - return WorkflowComment( - created_by=created_by, - resolved_by=resolved_by, - content="hello", - position_x=1, - position_y=2, - tenant_id=str(uuid4()), - app_id=str(uuid4()), - ) - - -COMMENT_MODELS = (Account, WorkflowComment, WorkflowCommentReply, WorkflowCommentMention) - - -@pytest.mark.parametrize("sqlite_session", [COMMENT_MODELS], indirect=True) -def test_workflow_comment_account_accessors_and_cache(sqlite_session: Session) -> None: - created_account = _account("Creator") - resolved_account = _account("Resolver") - comment = _comment(created_account.id, resolved_account.id) - sqlite_session.add_all([created_account, resolved_account, comment]) - sqlite_session.commit() - - assert comment.created_by_account(sqlite_session) is created_account - assert comment.resolved_by_account(sqlite_session) is resolved_account - - comment.cache_created_by_account(created_account) - comment.cache_resolved_by_account(resolved_account) - sqlite_session.delete(created_account) - sqlite_session.delete(resolved_account) - sqlite_session.commit() - - assert comment.created_by_account(sqlite_session) is created_account - assert comment.resolved_by_account(sqlite_session) is resolved_account - - comment_without_resolver = _comment(str(uuid4())) - sqlite_session.add(comment_without_resolver) - sqlite_session.commit() - assert comment_without_resolver.resolved_by_account(sqlite_session) is None - - -@pytest.mark.parametrize("sqlite_session", [COMMENT_MODELS], indirect=True) -def test_workflow_comment_counts_and_participants(sqlite_session: Session) -> None: - account_1 = _account("Creator") - account_2 = _account("Replier") - account_3 = _account("Mentioned") - missing_account_id = str(uuid4()) - comment = _comment(account_1.id) - comment.replies = [ - WorkflowCommentReply(comment_id=comment.id, content="reply-1", created_by=account_2.id), - WorkflowCommentReply(comment_id=comment.id, content="reply-2", created_by=account_2.id), - ] - comment.mentions = [ - WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=account_3.id), - WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=missing_account_id), - ] - sqlite_session.add_all([account_1, account_2, account_3, comment]) - sqlite_session.commit() - - participants = comment.participants(sqlite_session) - - assert comment.reply_count == 2 - assert comment.mention_count == 2 - assert {account.id for account in participants} == {account_1.id, account_2.id, account_3.id} - - -@pytest.mark.parametrize("sqlite_session", [COMMENT_MODELS], indirect=True) -def test_workflow_comment_participants_use_cached_accounts(sqlite_session: Session) -> None: - account_1 = _account("Creator") - account_2 = _account("Replier") - account_3 = _account("Mentioned") - comment = _comment(account_1.id) - reply = WorkflowCommentReply(comment_id=comment.id, content="reply-1", created_by=account_2.id) - mention = WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=account_3.id) - comment.replies = [reply] - comment.mentions = [mention] - sqlite_session.add_all([account_1, account_2, account_3, comment]) - sqlite_session.commit() - - comment.cache_created_by_account(account_1) - reply.cache_created_by_account(account_2) - mention.cache_mentioned_user_account(account_3) - sqlite_session.delete(account_1) - sqlite_session.delete(account_2) - sqlite_session.delete(account_3) - sqlite_session.commit() - - assert {account.id for account in comment.participants(sqlite_session)} == { - account_1.id, - account_2.id, - account_3.id, - } - - -@pytest.mark.parametrize("sqlite_session", [COMMENT_MODELS], indirect=True) -def test_reply_and_mention_account_accessors_and_cache(sqlite_session: Session) -> None: - reply_account = _account("Replier") - mention_account = _account("Mentioned") - comment = _comment(reply_account.id) - reply = WorkflowCommentReply(comment_id=comment.id, content="reply", created_by=reply_account.id) - mention = WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=mention_account.id) - comment.replies = [reply] - comment.mentions = [mention] - sqlite_session.add_all([reply_account, mention_account, comment]) - sqlite_session.commit() - - assert reply.created_by_account(sqlite_session) is reply_account - assert mention.mentioned_user_account(sqlite_session) is mention_account - - reply.cache_created_by_account(reply_account) - mention.cache_mentioned_user_account(mention_account) - sqlite_session.delete(reply_account) - sqlite_session.delete(mention_account) - sqlite_session.commit() - - assert reply.created_by_account(sqlite_session) is reply_account - assert mention.mentioned_user_account(sqlite_session) is mention_account diff --git a/api/tests/unit_tests/pyrefly.toml b/api/tests/unit_tests/pyrefly.toml index 5096ab70bd0..5423f9e5c7a 100644 --- a/api/tests/unit_tests/pyrefly.toml +++ b/api/tests/unit_tests/pyrefly.toml @@ -53,7 +53,6 @@ project-excludes = [ "controllers/console/app/test_statistic_api.py", "controllers/console/app/test_workflow.py", "controllers/console/app/test_workflow_app_log_api.py", - "controllers/console/app/test_workflow_comment_api.py", "controllers/console/app/test_workflow_convert_api.py", "controllers/console/app/test_workflow_node_output_inspector.py", "controllers/console/app/test_workflow_pause_details_api.py", @@ -759,7 +758,6 @@ project-excludes = [ "services/test_webhook_service_additional.py", "services/test_website_service.py", "services/test_workflow_collaboration_service.py", - "services/test_workflow_comment_service.py", "services/test_workflow_generator_service.py", "services/test_workflow_node_execution_trace_service.py", "services/test_workflow_run_service.py", diff --git a/api/tests/unit_tests/repositories/app/test_workflow_comment_repository.py b/api/tests/unit_tests/repositories/app/test_workflow_comment_repository.py new file mode 100644 index 00000000000..60f4dcff7d8 --- /dev/null +++ b/api/tests/unit_tests/repositories/app/test_workflow_comment_repository.py @@ -0,0 +1,766 @@ +"""Exercise workflow comment persistence with real short-lived Sessions.""" + +from datetime import datetime +from unittest.mock import Mock, patch + +import pytest +from sqlalchemy import func, select +from sqlalchemy.orm import Session, sessionmaker + +from machinery.context import RequestContext +from models import App, TenantAccountJoin, WorkflowComment, WorkflowCommentMention, WorkflowCommentReply +from models.account import Account, TenantAccountRole +from models.model import AppMode +from repositories.app.workflow_comment_repository import WorkflowCommentRepository +from services.app import workflow_comment_mention_gateway +from services.app.workflow_comment_mention_gateway import CeleryWorkflowCommentMentionNotifier +from services.app.workflow_comment_service import ( + MentionContext, + MentionRecipient, + WorkflowCommentDraft, + WorkflowCommentEdit, + WorkflowCommentNotFoundError, + WorkflowCommentPermissionError, + WorkflowCommentReplyDraft, + WorkflowCommentReplyNotFoundError, + WorkflowCommentService, +) +from tests.unit_tests.model_factories import make_account, make_app +from tests.unit_tests.repositories.app.console_visibility import UNADDRESSABLE_IN_WORKSPACE, MakeUnaddressable + +TENANT_ID = "11111111-1111-1111-1111-111111111111" +OTHER_TENANT_ID = "11111111-1111-1111-1111-111111111112" +APP_ID = "22222222-2222-2222-2222-222222222222" +OTHER_APP_ID = "22222222-2222-2222-2222-222222222223" +OWNER_ID = "33333333-3333-3333-3333-333333333333" +USER_2_ID = "33333333-3333-3333-3333-333333333334" +USER_3_ID = "33333333-3333-3333-3333-333333333335" +USER_4_ID = "33333333-3333-3333-3333-333333333336" +OUTSIDER_ID = "33333333-3333-3333-3333-333333333337" + + +@pytest.fixture +def delay_mock(monkeypatch: pytest.MonkeyPatch) -> Mock: + """Stand in for the Celery broker at the mention email task boundary.""" + mock = Mock() + monkeypatch.setattr(workflow_comment_mention_gateway.send_workflow_comment_mention_email_task, "delay", mock) + return mock + + +@pytest.fixture +def comment_service(repository: WorkflowCommentRepository) -> WorkflowCommentService: + """The production composition: real repository and Celery notifier, with only task delivery mocked.""" + return WorkflowCommentService( + comments=repository, + notifier=CeleryWorkflowCommentMentionNotifier(), + console_web_url="https://console.example.com", + ) + + +def _context(account_id: str) -> RequestContext: + return RequestContext(request_id="request", trace_id=None, account_id=account_id, active_workspace_id=TENANT_ID) + + +@pytest.fixture +def repository(sqlite_session_factory: sessionmaker[Session]) -> WorkflowCommentRepository: + return WorkflowCommentRepository(session_factory=sqlite_session_factory) + + +def _account( + account_id: str, + *, + name: str = "Test User", + email: str = "user@example.com", + interface_language: str | None = "en-US", +) -> Account: + return make_account(account_id=account_id, name=name, email=email, interface_language=interface_language) + + +def _app(*, app_id: str = APP_ID, tenant_id: str = TENANT_ID, name: str = "My App") -> App: + return make_app( + app_id=app_id, + tenant_id=tenant_id, + name=name, + mode=AppMode.WORKFLOW, + icon_type=None, + enable_site=False, + enable_api=False, + created_by=OWNER_ID, + ) + + +def _comment( + *, + tenant_id: str = TENANT_ID, + app_id: str = APP_ID, + created_by: str = OWNER_ID, + content: str = "hello", + resolved: bool = False, + resolved_at: datetime | None = None, + resolved_by: str | None = None, +) -> WorkflowComment: + return WorkflowComment( + tenant_id=tenant_id, + app_id=app_id, + position_x=1.0, + position_y=2.0, + content=content, + created_by=created_by, + resolved=resolved, + resolved_at=resolved_at, + resolved_by=resolved_by, + ) + + +def _membership(account_id: str, *, tenant_id: str = TENANT_ID) -> TenantAccountJoin: + return TenantAccountJoin(tenant_id=tenant_id, account_id=account_id, role=TenantAccountRole.NORMAL) + + +def _persist(session: Session, *objects: object) -> None: + session.add_all(objects) + session.commit() + + +def _mention_user_ids(session: Session, comment_id: str, *, reply_id: str | None = None) -> list[str]: + stmt = select(WorkflowCommentMention.mentioned_user_id).where(WorkflowCommentMention.comment_id == comment_id) + stmt = stmt.where( + WorkflowCommentMention.reply_id.is_(None) if reply_id is None else WorkflowCommentMention.reply_id == reply_id + ) + return list(session.scalars(stmt).all()) + + +def test_app_exists_is_scoped_to_workspace(sqlite_session: Session, repository: WorkflowCommentRepository) -> None: + _persist(sqlite_session, _app()) + + assert repository.app_exists(workspace_id=TENANT_ID, app_id=APP_ID) + assert not repository.app_exists(workspace_id=OTHER_TENANT_ID, app_id=APP_ID) + assert not repository.app_exists(workspace_id=TENANT_ID, app_id=OTHER_APP_ID) + + +@pytest.mark.parametrize("make_unaddressable", UNADDRESSABLE_IN_WORKSPACE) +def test_app_exists_hides_unaddressable_apps( + sqlite_session: Session, + sqlite_session_factory: sessionmaker[Session], + repository: WorkflowCommentRepository, + make_unaddressable: MakeUnaddressable, +) -> None: + _persist(sqlite_session, _app()) + with sqlite_session_factory.begin() as session: + make_unaddressable(session, APP_ID) + + assert not repository.app_exists(workspace_id=TENANT_ID, app_id=APP_ID) + + +def test_list_comments_resolves_accounts_and_participants( + sqlite_session: Session, repository: WorkflowCommentRepository +) -> None: + comment = _comment(resolved=True, resolved_by=USER_2_ID) + other_app_comment = _comment(app_id=OTHER_APP_ID) + _persist( + sqlite_session, + _account(OWNER_ID, name="Owner"), + _account(USER_2_ID, name="Resolver"), + _account(USER_3_ID, name="Replier"), + _account(USER_4_ID, name="Mentioned"), + comment, + other_app_comment, + ) + _persist( + sqlite_session, + WorkflowCommentReply(comment_id=comment.id, content="reply", created_by=USER_3_ID), + WorkflowCommentReply(comment_id=comment.id, content="again", created_by=OWNER_ID), + WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=USER_4_ID), + WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=OUTSIDER_ID), + ) + + [summary] = repository.list_comments(workspace_id=TENANT_ID, app_id=APP_ID) + + assert summary.id == comment.id + assert summary.created_by_account is not None + assert summary.created_by_account.name == "Owner" + assert summary.resolved_by_account is not None + assert summary.resolved_by_account.name == "Resolver" + assert summary.reply_count == 2 + assert summary.mention_count == 2 + # Creator, repliers, then mentioned users, each once; accounts that no longer exist are skipped. + assert [participant.id for participant in summary.participants] == [OWNER_ID, USER_3_ID, USER_4_ID] + + +def test_list_comments_orders_newest_first(sqlite_session: Session, repository: WorkflowCommentRepository) -> None: + older = _comment(content="older") + newer = _comment(content="newer") + _persist(sqlite_session, older, newer) + older.created_at = datetime(2024, 1, 1) + newer.created_at = datetime(2024, 1, 2) + sqlite_session.commit() + + comments = repository.list_comments(workspace_id=TENANT_ID, app_id=APP_ID) + + assert [comment.content for comment in comments] == ["newer", "older"] + + +def test_get_comment_returns_thread_with_accounts( + sqlite_session: Session, repository: WorkflowCommentRepository +) -> None: + comment = _comment() + _persist(sqlite_session, _account(OWNER_ID, name="Owner"), _account(USER_2_ID, name="Replier"), comment) + reply = WorkflowCommentReply(comment_id=comment.id, content="reply", created_by=USER_2_ID) + _persist(sqlite_session, reply) + _persist( + sqlite_session, + WorkflowCommentMention(comment_id=comment.id, reply_id=reply.id, mentioned_user_id=OWNER_ID), + ) + + detail = repository.get_comment(workspace_id=TENANT_ID, app_id=APP_ID, comment_id=comment.id) + + assert detail.created_by_account is not None + assert detail.created_by_account.id == OWNER_ID + assert detail.resolved_by_account is None + [reply_record] = detail.replies + assert reply_record.content == "reply" + assert reply_record.created_by_account is not None + assert reply_record.created_by_account.name == "Replier" + [mention_record] = detail.mentions + assert mention_record.reply_id == reply.id + assert mention_record.mentioned_user_account is not None + assert mention_record.mentioned_user_account.id == OWNER_ID + + +@pytest.mark.parametrize( + ("workspace_id", "app_id"), + [(OTHER_TENANT_ID, APP_ID), (TENANT_ID, OTHER_APP_ID)], + ids=["other-workspace", "other-app"], +) +def test_get_comment_is_scoped_to_workspace_and_app( + sqlite_session: Session, repository: WorkflowCommentRepository, workspace_id: str, app_id: str +) -> None: + comment = _comment() + _persist(sqlite_session, comment) + + with pytest.raises(WorkflowCommentNotFoundError): + repository.get_comment(workspace_id=workspace_id, app_id=app_id, comment_id=comment.id) + + +def test_insert_comment_keeps_only_workspace_member_mentions( + sqlite_session: Session, repository: WorkflowCommentRepository +) -> None: + _persist( + sqlite_session, + _app(name="Canvas"), + _account(OWNER_ID, name="Owner"), + _account(USER_2_ID, name="Member", email="member@example.com", interface_language="ja-JP"), + _account(USER_3_ID, name="Outsider", email="outsider@example.com"), + _membership(OWNER_ID), + _membership(USER_2_ID), + _membership(USER_3_ID, tenant_id=OTHER_TENANT_ID), + ) + + write = repository.insert_comment( + workspace_id=TENANT_ID, + app_id=APP_ID, + actor_id=OWNER_ID, + draft=WorkflowCommentDraft( + content="hello", position_x=1.0, position_y=2.0, mentioned_user_ids=(OWNER_ID, USER_3_ID, USER_2_ID) + ), + ) + + comment = sqlite_session.get_one(WorkflowComment, write.result.id) + assert comment.content == "hello" + assert comment.created_at == write.result.created_at + # The actor's self-mention is stored but never notified. + assert _mention_user_ids(sqlite_session, comment.id) == [OWNER_ID, USER_2_ID] + assert write.mentions == MentionContext( + app_name="Canvas", + commenter_name="Owner", + recipients=(MentionRecipient(email="member@example.com", name="Member", interface_language="ja-JP"),), + ) + + +def test_insert_comment_skips_recipients_without_email( + sqlite_session: Session, repository: WorkflowCommentRepository +) -> None: + _persist( + sqlite_session, + _app(), + _account(OWNER_ID), + _account(USER_2_ID, email=""), + _membership(USER_2_ID), + ) + + write = repository.insert_comment( + workspace_id=TENANT_ID, + app_id=APP_ID, + actor_id=OWNER_ID, + draft=WorkflowCommentDraft(content="hello", position_x=0, position_y=0, mentioned_user_ids=(USER_2_ID,)), + ) + + assert _mention_user_ids(sqlite_session, write.result.id) == [USER_2_ID] + assert write.mentions.recipients == () + + +def test_update_comment_raises_not_found(repository: WorkflowCommentRepository) -> None: + with pytest.raises(WorkflowCommentNotFoundError): + repository.update_comment( + workspace_id=TENANT_ID, + app_id=APP_ID, + comment_id="missing", + actor_id=OWNER_ID, + edit=WorkflowCommentEdit(content="x", position_x=None, position_y=None, mentioned_user_ids=None), + ) + + +def test_update_comment_rejects_non_creator(sqlite_session: Session, repository: WorkflowCommentRepository) -> None: + comment = _comment(created_by=OWNER_ID) + _persist(sqlite_session, comment) + + with pytest.raises(WorkflowCommentPermissionError, match="Only the comment creator can update it"): + repository.update_comment( + workspace_id=TENANT_ID, + app_id=APP_ID, + comment_id=comment.id, + actor_id=OUTSIDER_ID, + edit=WorkflowCommentEdit(content="x", position_x=None, position_y=None, mentioned_user_ids=None), + ) + + +def test_update_comment_replaces_mentions_and_reports_only_new_ones( + sqlite_session: Session, repository: WorkflowCommentRepository +) -> None: + comment = _comment() + _persist( + sqlite_session, + _app(), + _account(OWNER_ID, name="Owner"), + _account(USER_2_ID, name="Existing", email="existing@example.com"), + _account(USER_3_ID, name="New User", email="new@example.com"), + _account(USER_4_ID, name="Dropped", email="dropped@example.com"), + _membership(USER_2_ID), + _membership(USER_3_ID), + _membership(USER_4_ID), + comment, + ) + _persist( + sqlite_session, + WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=USER_2_ID), + WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=USER_4_ID), + ) + + write = repository.update_comment( + workspace_id=TENANT_ID, + app_id=APP_ID, + comment_id=comment.id, + actor_id=OWNER_ID, + edit=WorkflowCommentEdit( + content="updated", position_x=10.5, position_y=None, mentioned_user_ids=(USER_2_ID, USER_3_ID) + ), + ) + + sqlite_session.expire_all() + persisted = sqlite_session.get_one(WorkflowComment, comment.id) + assert persisted.content == "updated" + assert persisted.position_x == 10.5 + assert persisted.position_y == 2.0 + assert write.result.updated_at == persisted.updated_at + assert sorted(_mention_user_ids(sqlite_session, comment.id)) == [USER_2_ID, USER_3_ID] + assert [recipient.email for recipient in write.mentions.recipients] == ["new@example.com"] + + +def test_update_comment_keeps_mentions_when_list_omitted( + sqlite_session: Session, repository: WorkflowCommentRepository +) -> None: + comment = _comment() + _persist(sqlite_session, comment) + _persist(sqlite_session, WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=USER_2_ID)) + + write = repository.update_comment( + workspace_id=TENANT_ID, + app_id=APP_ID, + comment_id=comment.id, + actor_id=OWNER_ID, + edit=WorkflowCommentEdit(content="updated", position_x=None, position_y=None, mentioned_user_ids=None), + ) + + assert _mention_user_ids(sqlite_session, comment.id) == [USER_2_ID] + assert write.mentions.recipients == () + + +def test_update_comment_clears_mentions_with_empty_list( + sqlite_session: Session, repository: WorkflowCommentRepository +) -> None: + comment = _comment() + _persist(sqlite_session, comment) + reply = WorkflowCommentReply(comment_id=comment.id, content="reply", created_by=USER_2_ID) + _persist(sqlite_session, reply) + _persist( + sqlite_session, + WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=USER_2_ID), + WorkflowCommentMention(comment_id=comment.id, reply_id=reply.id, mentioned_user_id=USER_3_ID), + ) + + repository.update_comment( + workspace_id=TENANT_ID, + app_id=APP_ID, + comment_id=comment.id, + actor_id=OWNER_ID, + edit=WorkflowCommentEdit(content="updated", position_x=None, position_y=None, mentioned_user_ids=()), + ) + + assert _mention_user_ids(sqlite_session, comment.id) == [] + # Reply mentions belong to the reply and survive a comment mention update. + assert _mention_user_ids(sqlite_session, comment.id, reply_id=reply.id) == [USER_3_ID] + + +def test_delete_comment_rejects_non_creator(sqlite_session: Session, repository: WorkflowCommentRepository) -> None: + comment = _comment(created_by=OWNER_ID) + _persist(sqlite_session, comment) + + with pytest.raises(WorkflowCommentPermissionError, match="Only the comment creator can delete it"): + repository.delete_comment(workspace_id=TENANT_ID, app_id=APP_ID, comment_id=comment.id, actor_id=OUTSIDER_ID) + + assert sqlite_session.get(WorkflowComment, comment.id) is not None + + +def test_delete_comment_removes_replies_and_mentions( + sqlite_session: Session, repository: WorkflowCommentRepository +) -> None: + comment = _comment() + _persist(sqlite_session, comment) + reply = WorkflowCommentReply(comment_id=comment.id, content="reply", created_by=OWNER_ID) + _persist(sqlite_session, reply) + _persist( + sqlite_session, + WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=USER_2_ID), + WorkflowCommentMention(comment_id=comment.id, reply_id=reply.id, mentioned_user_id=USER_3_ID), + ) + comment_id = comment.id + + repository.delete_comment(workspace_id=TENANT_ID, app_id=APP_ID, comment_id=comment_id, actor_id=OWNER_ID) + + sqlite_session.expire_all() + assert sqlite_session.get(WorkflowComment, comment_id) is None + assert sqlite_session.scalar(select(func.count(WorkflowCommentReply.id))) == 0 + assert sqlite_session.scalar(select(func.count(WorkflowCommentMention.id))) == 0 + + +def test_resolve_comment_sets_fields(sqlite_session: Session, repository: WorkflowCommentRepository) -> None: + comment = _comment() + _persist(sqlite_session, comment) + now = datetime(2024, 1, 1, 12, 0, 0) + + with patch("repositories.app.workflow_comment_repository.naive_utc_now", return_value=now): + result = repository.resolve_comment( + workspace_id=TENANT_ID, app_id=APP_ID, comment_id=comment.id, actor_id=USER_2_ID + ) + + assert (result.resolved, result.resolved_at, result.resolved_by) == (True, now, USER_2_ID) + sqlite_session.expire_all() + persisted = sqlite_session.get_one(WorkflowComment, comment.id) + assert (persisted.resolved, persisted.resolved_at, persisted.resolved_by) == (True, now, USER_2_ID) + + +def test_resolve_comment_keeps_an_existing_resolution( + sqlite_session: Session, repository: WorkflowCommentRepository +) -> None: + resolved_at = datetime(2024, 1, 1, 10, 0, 0) + comment = _comment(resolved=True, resolved_at=resolved_at, resolved_by=USER_2_ID) + _persist(sqlite_session, comment) + + result = repository.resolve_comment(workspace_id=TENANT_ID, app_id=APP_ID, comment_id=comment.id, actor_id=OWNER_ID) + + assert (result.resolved_at, result.resolved_by) == (resolved_at, USER_2_ID) + + +def test_insert_reply_requires_comment_in_scope(sqlite_session: Session, repository: WorkflowCommentRepository) -> None: + comment = _comment(app_id=OTHER_APP_ID) + _persist(sqlite_session, comment) + + with pytest.raises(WorkflowCommentNotFoundError): + repository.insert_reply( + workspace_id=TENANT_ID, + app_id=APP_ID, + comment_id=comment.id, + actor_id=OWNER_ID, + draft=WorkflowCommentReplyDraft(content="hello", mentioned_user_ids=()), + ) + + +def test_insert_reply_creates_reply_mentions(sqlite_session: Session, repository: WorkflowCommentRepository) -> None: + comment = _comment() + _persist( + sqlite_session, + _app(), + _account(OWNER_ID), + _account(USER_2_ID, email="member@example.com"), + _membership(USER_2_ID), + comment, + ) + + write = repository.insert_reply( + workspace_id=TENANT_ID, + app_id=APP_ID, + comment_id=comment.id, + actor_id=OWNER_ID, + draft=WorkflowCommentReplyDraft(content="hello", mentioned_user_ids=(USER_2_ID, OUTSIDER_ID)), + ) + + reply = sqlite_session.get_one(WorkflowCommentReply, write.result.id) + assert reply.content == "hello" + assert reply.created_at == write.result.created_at + assert _mention_user_ids(sqlite_session, comment.id, reply_id=reply.id) == [USER_2_ID] + assert [recipient.email for recipient in write.mentions.recipients] == ["member@example.com"] + + +def test_update_reply_reports_missing_comment_before_missing_reply(repository: WorkflowCommentRepository) -> None: + with pytest.raises(WorkflowCommentNotFoundError): + repository.update_reply( + workspace_id=TENANT_ID, + app_id=APP_ID, + comment_id="missing-comment", + reply_id="missing-reply", + actor_id=OWNER_ID, + draft=WorkflowCommentReplyDraft(content="x", mentioned_user_ids=()), + ) + + +def test_update_reply_raises_not_found_for_reply_in_another_thread( + sqlite_session: Session, repository: WorkflowCommentRepository +) -> None: + comment = _comment() + other_comment = _comment() + _persist(sqlite_session, comment, other_comment) + reply = WorkflowCommentReply(comment_id=other_comment.id, content="reply", created_by=OWNER_ID) + _persist(sqlite_session, reply) + + with pytest.raises(WorkflowCommentReplyNotFoundError): + repository.update_reply( + workspace_id=TENANT_ID, + app_id=APP_ID, + comment_id=comment.id, + reply_id=reply.id, + actor_id=OWNER_ID, + draft=WorkflowCommentReplyDraft(content="x", mentioned_user_ids=()), + ) + + sqlite_session.expire_all() + assert sqlite_session.get_one(WorkflowCommentReply, reply.id).content == "reply" + + +def test_update_reply_rejects_non_creator(sqlite_session: Session, repository: WorkflowCommentRepository) -> None: + comment = _comment() + _persist(sqlite_session, comment) + reply = WorkflowCommentReply(comment_id=comment.id, content="reply", created_by=OWNER_ID) + _persist(sqlite_session, reply) + + with pytest.raises(WorkflowCommentPermissionError, match="Only the reply creator can update it"): + repository.update_reply( + workspace_id=TENANT_ID, + app_id=APP_ID, + comment_id=comment.id, + reply_id=reply.id, + actor_id=OUTSIDER_ID, + draft=WorkflowCommentReplyDraft(content="x", mentioned_user_ids=()), + ) + + +def test_update_reply_replaces_mentions_and_reports_only_new_ones( + sqlite_session: Session, repository: WorkflowCommentRepository +) -> None: + comment = _comment() + _persist( + sqlite_session, + _app(), + _account(OWNER_ID), + _account(USER_2_ID, email="existing@example.com"), + _account(USER_3_ID, email="new@example.com"), + _membership(USER_2_ID), + _membership(USER_3_ID), + comment, + ) + reply = WorkflowCommentReply(comment_id=comment.id, content="old", created_by=OWNER_ID) + _persist(sqlite_session, reply) + _persist( + sqlite_session, + WorkflowCommentMention(comment_id=comment.id, reply_id=reply.id, mentioned_user_id=USER_2_ID), + ) + + write = repository.update_reply( + workspace_id=TENANT_ID, + app_id=APP_ID, + comment_id=comment.id, + reply_id=reply.id, + actor_id=OWNER_ID, + draft=WorkflowCommentReplyDraft(content="new", mentioned_user_ids=(USER_2_ID, USER_3_ID)), + ) + + sqlite_session.expire_all() + persisted = sqlite_session.get_one(WorkflowCommentReply, reply.id) + assert persisted.content == "new" + assert write.result.updated_at == persisted.updated_at + assert sorted(_mention_user_ids(sqlite_session, comment.id, reply_id=reply.id)) == [USER_2_ID, USER_3_ID] + assert [recipient.email for recipient in write.mentions.recipients] == ["new@example.com"] + + +def test_delete_reply_rejects_non_creator(sqlite_session: Session, repository: WorkflowCommentRepository) -> None: + comment = _comment() + _persist(sqlite_session, comment) + reply = WorkflowCommentReply(comment_id=comment.id, content="reply", created_by=OWNER_ID) + _persist(sqlite_session, reply) + + with pytest.raises(WorkflowCommentPermissionError, match="Only the reply creator can delete it"): + repository.delete_reply( + workspace_id=TENANT_ID, app_id=APP_ID, comment_id=comment.id, reply_id=reply.id, actor_id=OUTSIDER_ID + ) + + +def test_delete_reply_raises_not_found(sqlite_session: Session, repository: WorkflowCommentRepository) -> None: + comment = _comment() + _persist(sqlite_session, comment) + + with pytest.raises(WorkflowCommentReplyNotFoundError): + repository.delete_reply( + workspace_id=TENANT_ID, app_id=APP_ID, comment_id=comment.id, reply_id="missing", actor_id=OWNER_ID + ) + + +def test_delete_reply_removes_its_mentions(sqlite_session: Session, repository: WorkflowCommentRepository) -> None: + comment = _comment() + _persist(sqlite_session, comment) + reply = WorkflowCommentReply(comment_id=comment.id, content="reply", created_by=OWNER_ID) + _persist(sqlite_session, reply) + _persist( + sqlite_session, + WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=USER_2_ID), + WorkflowCommentMention(comment_id=comment.id, reply_id=reply.id, mentioned_user_id=USER_3_ID), + ) + reply_id = reply.id + + repository.delete_reply( + workspace_id=TENANT_ID, app_id=APP_ID, comment_id=comment.id, reply_id=reply_id, actor_id=OWNER_ID + ) + + sqlite_session.expire_all() + assert sqlite_session.get(WorkflowCommentReply, reply_id) is None + assert _mention_user_ids(sqlite_session, comment.id, reply_id=reply_id) == [] + assert _mention_user_ids(sqlite_session, comment.id) == [USER_2_ID] + + +def test_update_comment_enqueues_mention_email_only_for_new_mentions( + sqlite_session: Session, comment_service: WorkflowCommentService, delay_mock: Mock +) -> None: + comment = _comment() + _persist( + sqlite_session, + _app(name="Canvas"), + _account(OWNER_ID, name="Owner", email="owner@example.com"), + _account(USER_2_ID, name="Existing", email="existing@example.com"), + _account(USER_3_ID, name="New User", email="new@example.com", interface_language="zh-Hans"), + _membership(USER_2_ID), + _membership(USER_3_ID), + comment, + ) + _persist(sqlite_session, WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=USER_2_ID)) + + comment_service.update_comment( + _context(OWNER_ID), + APP_ID, + comment.id, + WorkflowCommentEdit( + content="updated", position_x=None, position_y=None, mentioned_user_ids=(USER_2_ID, USER_3_ID) + ), + ) + + delay_mock.assert_called_once_with( + language="zh-Hans", + to="new@example.com", + mentioned_name="New User", + commenter_name="Owner", + app_name="Canvas", + comment_content="updated", + app_url=f"https://console.example.com/app/{APP_ID}/workflow", + ) + assert sorted(_mention_user_ids(sqlite_session, comment.id)) == [USER_2_ID, USER_3_ID] + + +def test_update_comment_without_mention_list_enqueues_nothing( + sqlite_session: Session, comment_service: WorkflowCommentService, delay_mock: Mock +) -> None: + comment = _comment() + _persist(sqlite_session, _app(), _account(USER_2_ID, email="existing@example.com"), _membership(USER_2_ID), comment) + _persist(sqlite_session, WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=USER_2_ID)) + + comment_service.update_comment( + _context(OWNER_ID), + APP_ID, + comment.id, + WorkflowCommentEdit(content="updated", position_x=None, position_y=None, mentioned_user_ids=None), + ) + + delay_mock.assert_not_called() + + +def test_create_comment_and_reply_enqueue_mention_emails_after_commit( + sqlite_session: Session, comment_service: WorkflowCommentService, delay_mock: Mock +) -> None: + _persist( + sqlite_session, + _app(), + _account(OWNER_ID, name="Owner"), + _account(USER_2_ID, name="Member", email="member@example.com"), + _membership(OWNER_ID), + _membership(USER_2_ID), + ) + + def committed_mentions(**_kwargs: str) -> None: + # The email is enqueued only once the mention row is visible to other sessions. + with Session(sqlite_session.get_bind()) as other_session: + assert (other_session.scalar(select(func.count(WorkflowCommentMention.id))) or 0) > 0 + + delay_mock.side_effect = committed_mentions + + created = comment_service.create_comment( + _context(OWNER_ID), + APP_ID, + WorkflowCommentDraft(content="hello", position_x=0, position_y=0, mentioned_user_ids=(OWNER_ID, USER_2_ID)), + ) + comment_service.create_reply( + _context(OWNER_ID), + APP_ID, + created.id, + WorkflowCommentReplyDraft(content="reply", mentioned_user_ids=(USER_2_ID,)), + ) + + # The self-mention is stored but never emailed. + assert [call.kwargs["to"] for call in delay_mock.call_args_list] == ["member@example.com", "member@example.com"] + assert [call.kwargs["comment_content"] for call in delay_mock.call_args_list] == ["hello", "reply"] + + +def test_update_reply_enqueues_mention_email_only_for_new_mentions( + sqlite_session: Session, comment_service: WorkflowCommentService, delay_mock: Mock +) -> None: + comment = _comment() + _persist( + sqlite_session, + _app(), + _account(OWNER_ID), + _account(USER_2_ID, email="existing@example.com"), + _account(USER_3_ID, email="new@example.com"), + _membership(USER_2_ID), + _membership(USER_3_ID), + comment, + ) + reply = WorkflowCommentReply(comment_id=comment.id, content="old", created_by=OWNER_ID) + _persist(sqlite_session, reply) + _persist( + sqlite_session, + WorkflowCommentMention(comment_id=comment.id, reply_id=reply.id, mentioned_user_id=USER_2_ID), + ) + + comment_service.update_reply( + _context(OWNER_ID), + APP_ID, + comment.id, + reply.id, + WorkflowCommentReplyDraft(content="new", mentioned_user_ids=(USER_2_ID, USER_3_ID)), + ) + + assert [call.kwargs["to"] for call in delay_mock.call_args_list] == ["new@example.com"] diff --git a/api/tests/unit_tests/services/app/test_workflow_comment_service.py b/api/tests/unit_tests/services/app/test_workflow_comment_service.py new file mode 100644 index 00000000000..fd8dea1e818 --- /dev/null +++ b/api/tests/unit_tests/services/app/test_workflow_comment_service.py @@ -0,0 +1,319 @@ +"""Workflow comment rules, isolated from persistence by an in-memory store.""" + +import datetime +from collections.abc import Callable, Sequence + +import pytest + +from machinery.context import RequestContext +from services.app.workflow_comment_service import ( + InvalidMentionedUserIdError, + InvalidWorkflowCommentContentError, + MentionContext, + MentionRecipient, + WorkflowCommentAppNotFoundError, + WorkflowCommentCreated, + WorkflowCommentDetail, + WorkflowCommentDraft, + WorkflowCommentEdit, + WorkflowCommentMentionNotification, + WorkflowCommentMentionNotifier, + WorkflowCommentReplyDraft, + WorkflowCommentResolved, + WorkflowCommentService, + WorkflowCommentStore, + WorkflowCommentSummary, + WorkflowCommentUpdated, + WorkflowCommentWrite, + format_comment_excerpt, +) + +CONTEXT = RequestContext("request", None, "account-1", "workspace-1") +APP_ID = "app-1" +NOW = datetime.datetime(2024, 1, 1, 12, 0, 0) +USER_A = "33333333-3333-3333-3333-333333333333" +USER_B = "33333333-3333-3333-3333-333333333334" +NO_MENTIONS = MentionContext(app_name=None, commenter_name=None, recipients=()) + + +class InMemoryStore(WorkflowCommentStore): + """Records every scoped call and returns canned writes.""" + + def __init__(self, *, app_visible: bool = True, mentions: MentionContext = NO_MENTIONS) -> None: + self.app_visible = app_visible + self.mentions = mentions + self.calls: list[tuple[str, dict[str, object]]] = [] + + def _record(self, name: str, **kwargs: object) -> None: + assert kwargs.pop("workspace_id") == CONTEXT.active_workspace_id + assert kwargs.pop("app_id") == APP_ID + if "actor_id" in kwargs: + assert kwargs.pop("actor_id") == CONTEXT.account_id + self.calls.append((name, kwargs)) + + def app_exists(self, *, workspace_id: str, app_id: str) -> bool: + assert (workspace_id, app_id) == (CONTEXT.active_workspace_id, APP_ID) + return self.app_visible + + def list_comments(self, *, workspace_id: str, app_id: str) -> Sequence[WorkflowCommentSummary]: + self._record("list_comments", workspace_id=workspace_id, app_id=app_id) + return [] + + def get_comment(self, *, workspace_id: str, app_id: str, comment_id: str) -> WorkflowCommentDetail: + self._record("get_comment", workspace_id=workspace_id, app_id=app_id, comment_id=comment_id) + raise NotImplementedError + + def insert_comment( + self, *, workspace_id: str, app_id: str, actor_id: str, draft: WorkflowCommentDraft + ) -> WorkflowCommentWrite[WorkflowCommentCreated]: + self._record("insert_comment", workspace_id=workspace_id, app_id=app_id, actor_id=actor_id, draft=draft) + return WorkflowCommentWrite(WorkflowCommentCreated(id="comment-1", created_at=NOW), self.mentions) + + def update_comment( + self, *, workspace_id: str, app_id: str, comment_id: str, actor_id: str, edit: WorkflowCommentEdit + ) -> WorkflowCommentWrite[WorkflowCommentUpdated]: + self._record( + "update_comment", + workspace_id=workspace_id, + app_id=app_id, + comment_id=comment_id, + actor_id=actor_id, + edit=edit, + ) + return WorkflowCommentWrite(WorkflowCommentUpdated(id=comment_id, updated_at=NOW), self.mentions) + + def delete_comment(self, *, workspace_id: str, app_id: str, comment_id: str, actor_id: str) -> None: + self._record( + "delete_comment", workspace_id=workspace_id, app_id=app_id, comment_id=comment_id, actor_id=actor_id + ) + + def resolve_comment( + self, *, workspace_id: str, app_id: str, comment_id: str, actor_id: str + ) -> WorkflowCommentResolved: + self._record( + "resolve_comment", workspace_id=workspace_id, app_id=app_id, comment_id=comment_id, actor_id=actor_id + ) + return WorkflowCommentResolved(id=comment_id, resolved=True, resolved_at=NOW, resolved_by=actor_id) + + def insert_reply( + self, *, workspace_id: str, app_id: str, comment_id: str, actor_id: str, draft: WorkflowCommentReplyDraft + ) -> WorkflowCommentWrite[WorkflowCommentCreated]: + self._record( + "insert_reply", + workspace_id=workspace_id, + app_id=app_id, + comment_id=comment_id, + actor_id=actor_id, + draft=draft, + ) + return WorkflowCommentWrite(WorkflowCommentCreated(id="reply-1", created_at=NOW), self.mentions) + + def update_reply( + self, + *, + workspace_id: str, + app_id: str, + comment_id: str, + reply_id: str, + actor_id: str, + draft: WorkflowCommentReplyDraft, + ) -> WorkflowCommentWrite[WorkflowCommentUpdated]: + self._record( + "update_reply", + workspace_id=workspace_id, + app_id=app_id, + comment_id=comment_id, + reply_id=reply_id, + actor_id=actor_id, + draft=draft, + ) + return WorkflowCommentWrite(WorkflowCommentUpdated(id=reply_id, updated_at=NOW), self.mentions) + + def delete_reply(self, *, workspace_id: str, app_id: str, comment_id: str, reply_id: str, actor_id: str) -> None: + self._record( + "delete_reply", + workspace_id=workspace_id, + app_id=app_id, + comment_id=comment_id, + reply_id=reply_id, + actor_id=actor_id, + ) + + +class RecordingNotifier(WorkflowCommentMentionNotifier): + def __init__(self) -> None: + self.sent: list[WorkflowCommentMentionNotification] = [] + + def notify(self, notifications: Sequence[WorkflowCommentMentionNotification]) -> None: + self.sent.extend(notifications) + + +def _service(store: InMemoryStore, notifier: RecordingNotifier | None = None) -> WorkflowCommentService: + return WorkflowCommentService( + comments=store, notifier=notifier or RecordingNotifier(), console_web_url="https://console.example.com/" + ) + + +COMMENT_DRAFT = WorkflowCommentDraft(content="hello", position_x=1.0, position_y=2.0, mentioned_user_ids=()) +REPLY_DRAFT = WorkflowCommentReplyDraft(content="hello", mentioned_user_ids=()) +COMMENT_EDIT = WorkflowCommentEdit(content="hello", position_x=None, position_y=None, mentioned_user_ids=None) + +type Operation = Callable[[WorkflowCommentService], object] + +OPERATIONS: list[Operation] = [ + lambda service: service.ensure_app(CONTEXT, APP_ID), + lambda service: service.list_comments(CONTEXT, APP_ID), + lambda service: service.get_comment(CONTEXT, APP_ID, "comment-1"), + lambda service: service.create_comment(CONTEXT, APP_ID, COMMENT_DRAFT), + lambda service: service.update_comment(CONTEXT, APP_ID, "comment-1", COMMENT_EDIT), + lambda service: service.delete_comment(CONTEXT, APP_ID, "comment-1"), + lambda service: service.resolve_comment(CONTEXT, APP_ID, "comment-1"), + lambda service: service.create_reply(CONTEXT, APP_ID, "comment-1", REPLY_DRAFT), + lambda service: service.update_reply(CONTEXT, APP_ID, "comment-1", "reply-1", REPLY_DRAFT), + lambda service: service.delete_reply(CONTEXT, APP_ID, "comment-1", "reply-1"), +] + + +@pytest.mark.parametrize("operation", OPERATIONS) +def test_every_operation_requires_an_addressable_app(operation: Operation) -> None: + store = InMemoryStore(app_visible=False) + + with pytest.raises(WorkflowCommentAppNotFoundError): + operation(_service(store)) + + assert store.calls == [] + + +CONTENT_WRITES: list[Callable[[WorkflowCommentService, str], object]] = [ + lambda service, content: service.create_comment( + CONTEXT, APP_ID, WorkflowCommentDraft(content=content, position_x=0, position_y=0, mentioned_user_ids=()) + ), + lambda service, content: service.update_comment( + CONTEXT, + APP_ID, + "comment-1", + WorkflowCommentEdit(content=content, position_x=None, position_y=None, mentioned_user_ids=None), + ), + lambda service, content: service.create_reply( + CONTEXT, APP_ID, "comment-1", WorkflowCommentReplyDraft(content=content, mentioned_user_ids=()) + ), + lambda service, content: service.update_reply( + CONTEXT, APP_ID, "comment-1", "reply-1", WorkflowCommentReplyDraft(content=content, mentioned_user_ids=()) + ), +] + + +@pytest.mark.parametrize("write", CONTENT_WRITES) +@pytest.mark.parametrize( + ("content", "message"), + [ + (" ", "Comment content cannot be empty"), + ("a" * 1001, "Comment content cannot exceed 1000 characters"), + ], + ids=["blank", "too-long"], +) +def test_invalid_content_is_rejected_before_persistence( + write: Callable[[WorkflowCommentService, str], object], content: str, message: str +) -> None: + store = InMemoryStore() + + with pytest.raises(InvalidWorkflowCommentContentError, match=message): + write(_service(store), content) + + assert store.calls == [] + + +def test_mentions_are_deduplicated_and_empty_ids_skipped() -> None: + store = InMemoryStore() + + _service(store).create_comment( + CONTEXT, + APP_ID, + WorkflowCommentDraft( + content="hello", position_x=0, position_y=0, mentioned_user_ids=(USER_A, "", USER_B, USER_A) + ), + ) + + [(_, kwargs)] = store.calls + draft = kwargs["draft"] + assert isinstance(draft, WorkflowCommentDraft) + assert draft.mentioned_user_ids == (USER_A, USER_B) + + +def test_malformed_mention_id_is_rejected() -> None: + store = InMemoryStore() + + with pytest.raises(InvalidMentionedUserIdError, match="not-a-uuid is not a valid uuid."): + _service(store).create_reply( + CONTEXT, APP_ID, "comment-1", WorkflowCommentReplyDraft(content="hi", mentioned_user_ids=("not-a-uuid",)) + ) + + assert store.calls == [] + + +def test_omitted_comment_mentions_stay_omitted() -> None: + store = InMemoryStore() + + _service(store).update_comment(CONTEXT, APP_ID, "comment-1", COMMENT_EDIT) + + [(_, kwargs)] = store.calls + edit = kwargs["edit"] + assert isinstance(edit, WorkflowCommentEdit) + assert edit.mentioned_user_ids is None + + +def test_notifications_are_built_from_committed_mentions_with_fallbacks() -> None: + store = InMemoryStore( + mentions=MentionContext( + app_name=None, + commenter_name=None, + recipients=( + MentionRecipient(email="ja@example.com", name="Hanako", interface_language="ja-JP"), + MentionRecipient(email="anon@example.com", name=None, interface_language=None), + ), + ) + ) + notifier = RecordingNotifier() + + _service(store, notifier).create_comment( + CONTEXT, + APP_ID, + WorkflowCommentDraft(content=" " + "x" * 250 + " ", position_x=0, position_y=0, mentioned_user_ids=()), + ) + + excerpt = "x" * 197 + "..." + app_url = "https://console.example.com/app/app-1/workflow" + assert notifier.sent == [ + WorkflowCommentMentionNotification( + language="ja-JP", + to="ja@example.com", + mentioned_name="Hanako", + commenter_name="Dify user", + app_name="Dify app", + comment_content=excerpt, + app_url=app_url, + ), + WorkflowCommentMentionNotification( + language="en-US", + to="anon@example.com", + mentioned_name="anon@example.com", + commenter_name="Dify user", + app_name="Dify app", + comment_content=excerpt, + app_url=app_url, + ), + ] + + +def test_no_notification_is_sent_without_recipients() -> None: + notifier = RecordingNotifier() + + _service(InMemoryStore(), notifier).create_reply(CONTEXT, APP_ID, "comment-1", REPLY_DRAFT) + + assert notifier.sent == [] + + +def test_format_comment_excerpt_handles_short_and_long_limits() -> None: + assert format_comment_excerpt(" hello ", max_length=10) == "hello" + assert format_comment_excerpt("abcdefghijk", max_length=3) == "abc" + assert format_comment_excerpt(" abcdefghijk ", max_length=8) == "abcde..." diff --git a/api/tests/unit_tests/services/test_workflow_comment_service.py b/api/tests/unit_tests/services/test_workflow_comment_service.py deleted file mode 100644 index 31b428ac1ab..00000000000 --- a/api/tests/unit_tests/services/test_workflow_comment_service.py +++ /dev/null @@ -1,703 +0,0 @@ -"""Persistence-focused tests for :mod:`services.workflow_comment_service`. - -The service opens its own sessions for most operations, so these tests bind it to the -same disposable SQLite engine used for fixture setup and assert committed database -state. External task dispatch and the clock remain mocked at their I/O boundaries. -""" - -from datetime import datetime -from types import SimpleNamespace -from unittest.mock import Mock, patch - -import pytest -from sqlalchemy import event, func, select -from sqlalchemy.orm import Session -from werkzeug.exceptions import Forbidden, NotFound - -from models import App, TenantAccountJoin, WorkflowComment, WorkflowCommentMention, WorkflowCommentReply -from models.account import Account, TenantAccountRole -from models.model import AppMode -from services import workflow_comment_service as service_module -from services.workflow_comment_service import WorkflowCommentService -from tests.unit_tests.model_factories import make_account, make_app - -TENANT_ID = "11111111-1111-1111-1111-111111111111" -OTHER_TENANT_ID = "11111111-1111-1111-1111-111111111112" -APP_ID = "22222222-2222-2222-2222-222222222222" -OTHER_APP_ID = "22222222-2222-2222-2222-222222222223" -OWNER_ID = "33333333-3333-3333-3333-333333333333" -USER_2_ID = "33333333-3333-3333-3333-333333333334" -USER_3_ID = "33333333-3333-3333-3333-333333333335" -USER_4_ID = "33333333-3333-3333-3333-333333333336" -OUTSIDER_ID = "33333333-3333-3333-3333-333333333337" - - -@pytest.fixture -def delay_mock(monkeypatch: pytest.MonkeyPatch) -> Mock: - mock = Mock() - monkeypatch.setattr(service_module.send_workflow_comment_mention_email_task, "delay", mock) - return mock - - -@pytest.fixture(autouse=True) -def bind_service_database( - sqlite_session: Session, - monkeypatch: pytest.MonkeyPatch, - delay_mock: Mock, -) -> None: - """Bind service-owned sessions to the engine prepared by the shared SQLite fixture.""" - monkeypatch.setattr(service_module, "db", SimpleNamespace(engine=sqlite_session.get_bind())) - - -def _account( - account_id: str, - *, - name: str = "Test User", - email: str = "user@example.com", - interface_language: str | None = "en-US", -) -> Account: - return make_account(account_id=account_id, name=name, email=email, interface_language=interface_language) - - -def _app(*, app_id: str = APP_ID, tenant_id: str = TENANT_ID, name: str = "My App") -> App: - return make_app( - app_id=app_id, - tenant_id=tenant_id, - name=name, - mode=AppMode.WORKFLOW, - icon_type=None, - enable_site=False, - enable_api=False, - created_by=OWNER_ID, - ) - - -def _comment( - *, - tenant_id: str = TENANT_ID, - app_id: str = APP_ID, - created_by: str = OWNER_ID, - content: str = "hello", - resolved: bool = False, - resolved_at: datetime | None = None, - resolved_by: str | None = None, -) -> WorkflowComment: - return WorkflowComment( - tenant_id=tenant_id, - app_id=app_id, - position_x=1.0, - position_y=2.0, - content=content, - created_by=created_by, - resolved=resolved, - resolved_at=resolved_at, - resolved_by=resolved_by, - ) - - -def _membership(account_id: str, *, tenant_id: str = TENANT_ID) -> TenantAccountJoin: - return TenantAccountJoin( - tenant_id=tenant_id, - account_id=account_id, - role=TenantAccountRole.NORMAL, - ) - - -def _persist(session: Session, *objects: object) -> None: - session.add_all(objects) - session.commit() - - -@pytest.mark.usefixtures("sqlite_session") -@pytest.mark.parametrize( - "sqlite_session", - [ - ( - Account, - TenantAccountJoin, - App, - WorkflowComment, - WorkflowCommentReply, - WorkflowCommentMention, - ) - ], - indirect=True, -) -class TestWorkflowCommentService: - def test_validate_content_rejects_empty(self) -> None: - with pytest.raises(ValueError): - WorkflowCommentService._validate_content(" ") - - def test_validate_content_rejects_too_long(self) -> None: - with pytest.raises(ValueError): - WorkflowCommentService._validate_content("a" * 1001) - - def test_filter_valid_mentioned_user_ids_filters_by_tenant_and_preserves_order( - self, sqlite_session: Session - ) -> None: - _persist( - sqlite_session, - _membership(OWNER_ID), - _membership(USER_2_ID), - _membership(USER_3_ID, tenant_id=OTHER_TENANT_ID), - ) - - result = WorkflowCommentService._filter_valid_mentioned_user_ids( - [ - OWNER_ID, - "", - 123, # type: ignore[list-item] - OWNER_ID, - USER_3_ID, - USER_2_ID, - ], - session=sqlite_session, - tenant_id=TENANT_ID, - ) - - assert result == [OWNER_ID, USER_2_ID] - - def test_format_comment_excerpt_handles_short_and_long_limits(self) -> None: - assert WorkflowCommentService._format_comment_excerpt(" hello ", max_length=10) == "hello" - assert WorkflowCommentService._format_comment_excerpt("abcdefghijk", max_length=3) == "abc" - assert WorkflowCommentService._format_comment_excerpt(" abcdefghijk ", max_length=8) == "abcde..." - - def test_build_mention_email_payloads_returns_empty_for_no_candidates(self, sqlite_session: Session) -> None: - assert ( - WorkflowCommentService._build_mention_email_payloads( - session=sqlite_session, - tenant_id=TENANT_ID, - app_id=APP_ID, - mentioner_id=OWNER_ID, - mentioned_user_ids=[], - content="hello", - ) - == [] - ) - assert ( - WorkflowCommentService._build_mention_email_payloads( - session=sqlite_session, - tenant_id=TENANT_ID, - app_id=APP_ID, - mentioner_id=OWNER_ID, - mentioned_user_ids=[OWNER_ID], - content="hello", - ) - == [] - ) - - def test_dispatch_mention_emails_enqueues_each_payload(self) -> None: - delay_mock = Mock() - with patch.object(service_module.send_workflow_comment_mention_email_task, "delay", delay_mock): - WorkflowCommentService._dispatch_mention_emails( - [ - {"to": "a@example.com"}, - {"to": "b@example.com"}, - ] - ) - - assert delay_mock.call_count == 2 - - def test_build_mention_email_payloads_skips_accounts_without_email(self, sqlite_session: Session) -> None: - _persist( - sqlite_session, - _app(), - _account(OWNER_ID, name="Commenter", email="commenter@example.com"), - _account(USER_2_ID, name="No Email", email=""), - _account(USER_3_ID, name="", email="user@example.com", interface_language=None), - _membership(USER_2_ID), - _membership(USER_3_ID), - ) - - payloads = WorkflowCommentService._build_mention_email_payloads( - session=sqlite_session, - tenant_id=TENANT_ID, - app_id=APP_ID, - mentioner_id=OWNER_ID, - mentioned_user_ids=[USER_2_ID, USER_3_ID], - content="hello", - ) - expected_app_url = f"{service_module.dify_config.CONSOLE_WEB_URL.rstrip('/')}/app/{APP_ID}/workflow" - - assert payloads == [ - { - "language": "en-US", - "to": "user@example.com", - "mentioned_name": "user@example.com", - "commenter_name": "Commenter", - "app_name": "My App", - "comment_content": "hello", - "app_url": expected_app_url, - } - ] - - def test_create_comment_creates_mentions(self, sqlite_session: Session) -> None: - _persist(sqlite_session, _membership(USER_2_ID)) - - result = WorkflowCommentService.create_comment( - tenant_id=TENANT_ID, - app_id=APP_ID, - created_by=OWNER_ID, - content="hello", - position_x=1.0, - position_y=2.0, - mentioned_user_ids=[USER_2_ID, OUTSIDER_ID], - ) - - comment = sqlite_session.get(WorkflowComment, result["id"]) - assert comment is not None - assert comment.content == "hello" - assert comment.created_at == result["created_at"] - mentions = sqlite_session.scalars( - select(WorkflowCommentMention).where(WorkflowCommentMention.comment_id == comment.id) - ).all() - assert [mention.mentioned_user_id for mention in mentions] == [USER_2_ID] - - def test_update_comment_raises_not_found(self, sqlite_session: Session) -> None: - with pytest.raises(NotFound): - WorkflowCommentService.update_comment( - tenant_id=TENANT_ID, - app_id=APP_ID, - comment_id="missing-comment", - user_id=OWNER_ID, - content="hello", - ) - - def test_update_comment_raises_forbidden(self, sqlite_session: Session) -> None: - comment = _comment() - _persist(sqlite_session, comment) - - with pytest.raises(Forbidden): - WorkflowCommentService.update_comment( - tenant_id=TENANT_ID, - app_id=APP_ID, - comment_id=comment.id, - user_id=OUTSIDER_ID, - content="hello", - ) - - def test_update_comment_replaces_mentions(self, sqlite_session: Session) -> None: - comment = _comment() - _persist(sqlite_session, comment) - _persist( - sqlite_session, - WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=USER_3_ID), - WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=USER_4_ID), - _membership(USER_2_ID), - ) - - result = WorkflowCommentService.update_comment( - tenant_id=TENANT_ID, - app_id=APP_ID, - comment_id=comment.id, - user_id=OWNER_ID, - content="updated", - mentioned_user_ids=[USER_2_ID, OUTSIDER_ID], - ) - - sqlite_session.expire_all() - persisted_comment = sqlite_session.get(WorkflowComment, comment.id) - assert persisted_comment is not None - assert persisted_comment.content == "updated" - assert result == {"id": comment.id, "updated_at": persisted_comment.updated_at} - mentions = sqlite_session.scalars( - select(WorkflowCommentMention).where(WorkflowCommentMention.comment_id == comment.id) - ).all() - assert [mention.mentioned_user_id for mention in mentions] == [USER_2_ID] - - def test_update_comment_preserves_mentions_when_mentioned_user_ids_omitted( - self, sqlite_session: Session, delay_mock: Mock - ) -> None: - comment = _comment() - _persist(sqlite_session, comment) - mention = WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=USER_2_ID) - _persist(sqlite_session, mention) - - WorkflowCommentService.update_comment( - tenant_id=TENANT_ID, - app_id=APP_ID, - comment_id=comment.id, - user_id=OWNER_ID, - content="updated", - ) - - sqlite_session.expire_all() - persisted_comment = sqlite_session.get(WorkflowComment, comment.id) - assert persisted_comment is not None - assert persisted_comment.content == "updated" - assert sqlite_session.get(WorkflowCommentMention, mention.id) is not None - delay_mock.assert_not_called() - - def test_update_comment_clears_mentions_when_empty_list_provided(self, sqlite_session: Session) -> None: - comment = _comment() - _persist(sqlite_session, comment) - _persist( - sqlite_session, - WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=USER_2_ID), - WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=USER_3_ID), - ) - - WorkflowCommentService.update_comment( - tenant_id=TENANT_ID, - app_id=APP_ID, - comment_id=comment.id, - user_id=OWNER_ID, - content="updated", - mentioned_user_ids=[], - ) - - mention_count = sqlite_session.scalar( - select(func.count()) - .select_from(WorkflowCommentMention) - .where(WorkflowCommentMention.comment_id == comment.id) - ) - assert mention_count == 0 - - def test_update_comment_notifies_only_new_mentions(self, sqlite_session: Session, delay_mock: Mock) -> None: - comment = _comment() - _persist( - sqlite_session, - _app(), - _account(OWNER_ID, name="Owner", email="owner@example.com"), - _account(USER_2_ID, name="Existing", email="existing@example.com"), - _account(USER_3_ID, name="New User", email="new@example.com"), - _membership(USER_2_ID), - _membership(USER_3_ID), - comment, - ) - _persist(sqlite_session, WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=USER_2_ID)) - - WorkflowCommentService.update_comment( - tenant_id=TENANT_ID, - app_id=APP_ID, - comment_id=comment.id, - user_id=OWNER_ID, - content="updated", - mentioned_user_ids=[USER_2_ID, USER_3_ID], - ) - - delay_mock.assert_called_once() - assert delay_mock.call_args.kwargs["to"] == "new@example.com" - mentions = sqlite_session.scalars( - select(WorkflowCommentMention).where(WorkflowCommentMention.comment_id == comment.id) - ).all() - assert {mention.mentioned_user_id for mention in mentions} == {USER_2_ID, USER_3_ID} - - def test_get_comments_preloads_related_accounts(self, sqlite_session: Session) -> None: - comment = _comment(resolved=True, resolved_by=USER_2_ID) - _persist( - sqlite_session, - _account(OWNER_ID, name="Owner"), - _account(USER_2_ID, name="Resolver"), - _account(USER_3_ID, name="Replier"), - _account(USER_4_ID, name="Mentioned"), - comment, - ) - reply = WorkflowCommentReply(comment_id=comment.id, content="reply", created_by=USER_3_ID) - mention = WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=USER_4_ID) - _persist(sqlite_session, reply, mention) - - result = WorkflowCommentService.get_comments(TENANT_ID, APP_ID) - - assert len(result) == 1 - loaded_comment = result[0] - assert loaded_comment.id == comment.id - created_by_account = loaded_comment.created_by_account(sqlite_session) - resolved_by_account = loaded_comment.resolved_by_account(sqlite_session) - reply_account = loaded_comment.replies[0].created_by_account(sqlite_session) - mentioned_user_account = loaded_comment.mentions[0].mentioned_user_account(sqlite_session) - assert created_by_account is not None - assert created_by_account.id == OWNER_ID - assert resolved_by_account is not None - assert resolved_by_account.id == USER_2_ID - assert reply_account is not None - assert reply_account.id == USER_3_ID - assert mentioned_user_account is not None - assert mentioned_user_account.id == USER_4_ID - - def test_preload_accounts_returns_early_for_empty_comments(self, sqlite_session: Session) -> None: - statements: list[str] = [] - bind = sqlite_session.get_bind() - - def record_sql(*args: object) -> None: - statements.append(str(args[2])) - - event.listen(bind, "before_cursor_execute", record_sql) - try: - WorkflowCommentService._preload_accounts(sqlite_session, []) - finally: - event.remove(bind, "before_cursor_execute", record_sql) - - assert statements == [] - - def test_get_comment_raises_not_found_with_provided_session(self, sqlite_session: Session) -> None: - with pytest.raises(NotFound): - WorkflowCommentService.get_comment(TENANT_ID, APP_ID, "missing-comment", session=sqlite_session) - - def test_get_comment_uses_context_manager_when_session_not_provided(self, sqlite_session: Session) -> None: - comment = _comment() - _persist(sqlite_session, _account(OWNER_ID, name="Owner"), comment) - - result = WorkflowCommentService.get_comment(TENANT_ID, APP_ID, comment.id) - - assert result.id == comment.id - created_by_account = result.created_by_account(sqlite_session) - assert created_by_account is not None - assert created_by_account.id == OWNER_ID - assert result.resolved_by_account(sqlite_session) is None - - def test_delete_comment_raises_forbidden(self, sqlite_session: Session) -> None: - comment = _comment() - _persist(sqlite_session, comment) - - with pytest.raises(Forbidden): - WorkflowCommentService.delete_comment(TENANT_ID, APP_ID, comment.id, OUTSIDER_ID) - - assert sqlite_session.get(WorkflowComment, comment.id) is not None - - def test_delete_comment_removes_related_entities(self, sqlite_session: Session) -> None: - comment = _comment() - _persist(sqlite_session, comment) - comment_id = comment.id - reply = WorkflowCommentReply(comment_id=comment.id, content="reply", created_by=USER_2_ID) - _persist(sqlite_session, reply) - _persist( - sqlite_session, - WorkflowCommentMention(comment_id=comment.id, mentioned_user_id=USER_3_ID), - WorkflowCommentMention(comment_id=comment.id, reply_id=reply.id, mentioned_user_id=USER_4_ID), - ) - - WorkflowCommentService.delete_comment(TENANT_ID, APP_ID, comment_id, OWNER_ID) - - sqlite_session.expire_all() - assert sqlite_session.get(WorkflowComment, comment_id) is None - assert ( - sqlite_session.scalar( - select(func.count()) - .select_from(WorkflowCommentReply) - .where(WorkflowCommentReply.comment_id == comment_id) - ) - == 0 - ) - assert ( - sqlite_session.scalar( - select(func.count()) - .select_from(WorkflowCommentMention) - .where(WorkflowCommentMention.comment_id == comment_id) - ) - == 0 - ) - - def test_resolve_comment_sets_fields(self, sqlite_session: Session) -> None: - comment = _comment() - _persist(sqlite_session, comment) - now = datetime(2026, 7, 10, 12, 0, 0) - - with patch.object(service_module, "naive_utc_now", return_value=now): - result = WorkflowCommentService.resolve_comment(TENANT_ID, APP_ID, comment.id, USER_2_ID) - - assert result.resolved is True - assert result.resolved_at == now - assert result.resolved_by == USER_2_ID - sqlite_session.expire_all() - persisted_comment = sqlite_session.get(WorkflowComment, comment.id) - assert persisted_comment is not None - assert persisted_comment.resolved is True - assert persisted_comment.resolved_at == now - assert persisted_comment.resolved_by == USER_2_ID - - def test_resolve_comment_noop_when_already_resolved(self, sqlite_session: Session) -> None: - resolved_at = datetime(2026, 7, 9, 12, 0, 0) - comment = _comment(resolved=True, resolved_at=resolved_at, resolved_by=USER_2_ID) - _persist(sqlite_session, comment) - - result = WorkflowCommentService.resolve_comment(TENANT_ID, APP_ID, comment.id, USER_3_ID) - - assert result.resolved_at == resolved_at - assert result.resolved_by == USER_2_ID - - def test_create_reply_requires_comment(self, sqlite_session: Session) -> None: - with pytest.raises(NotFound): - WorkflowCommentService.create_reply("missing-comment", "hello", OWNER_ID) - - def test_create_reply_creates_mentions(self, sqlite_session: Session) -> None: - comment = _comment() - _persist(sqlite_session, comment, _membership(USER_2_ID)) - - result = WorkflowCommentService.create_reply( - comment_id=comment.id, - content="hello", - created_by=OWNER_ID, - mentioned_user_ids=[USER_2_ID, OUTSIDER_ID], - ) - - reply = sqlite_session.get(WorkflowCommentReply, result["id"]) - assert reply is not None - assert reply.content == "hello" - assert reply.created_at == result["created_at"] - mentions = sqlite_session.scalars( - select(WorkflowCommentMention).where(WorkflowCommentMention.reply_id == reply.id) - ).all() - assert [mention.mentioned_user_id for mention in mentions] == [USER_2_ID] - - def test_update_reply_raises_not_found(self, sqlite_session: Session) -> None: - with pytest.raises(NotFound): - WorkflowCommentService.update_reply( - tenant_id=TENANT_ID, - app_id=APP_ID, - comment_id="missing-comment", - reply_id="missing-reply", - user_id=OWNER_ID, - content="hello", - ) - - def test_update_reply_raises_forbidden(self, sqlite_session: Session) -> None: - comment = _comment() - _persist(sqlite_session, comment) - reply = WorkflowCommentReply(comment_id=comment.id, content="reply", created_by=OWNER_ID) - _persist(sqlite_session, reply) - - with pytest.raises(Forbidden): - WorkflowCommentService.update_reply( - tenant_id=TENANT_ID, - app_id=APP_ID, - comment_id=comment.id, - reply_id=reply.id, - user_id=OUTSIDER_ID, - content="hello", - ) - - def test_update_reply_replaces_mentions(self, sqlite_session: Session) -> None: - comment = _comment() - _persist(sqlite_session, comment, _membership(USER_2_ID)) - reply = WorkflowCommentReply(comment_id=comment.id, content="reply", created_by=OWNER_ID) - _persist(sqlite_session, reply) - _persist( - sqlite_session, - WorkflowCommentMention(comment_id=comment.id, reply_id=reply.id, mentioned_user_id=USER_3_ID), - ) - - result = WorkflowCommentService.update_reply( - tenant_id=TENANT_ID, - app_id=APP_ID, - comment_id=comment.id, - reply_id=reply.id, - user_id=OWNER_ID, - content="new", - mentioned_user_ids=[USER_2_ID, OUTSIDER_ID], - ) - - sqlite_session.expire_all() - persisted_reply = sqlite_session.get(WorkflowCommentReply, reply.id) - assert persisted_reply is not None - assert persisted_reply.content == "new" - assert result == {"id": reply.id, "updated_at": persisted_reply.updated_at} - mentions = sqlite_session.scalars( - select(WorkflowCommentMention).where(WorkflowCommentMention.reply_id == reply.id) - ).all() - assert [mention.mentioned_user_id for mention in mentions] == [USER_2_ID] - - def test_update_comment_updates_position_coordinates_when_provided(self, sqlite_session: Session) -> None: - comment = _comment() - _persist(sqlite_session, comment) - - WorkflowCommentService.update_comment( - tenant_id=TENANT_ID, - app_id=APP_ID, - comment_id=comment.id, - user_id=OWNER_ID, - content="updated", - position_x=10.5, - position_y=20.5, - mentioned_user_ids=[], - ) - - sqlite_session.expire_all() - persisted_comment = sqlite_session.get(WorkflowComment, comment.id) - assert persisted_comment is not None - assert persisted_comment.position_x == 10.5 - assert persisted_comment.position_y == 20.5 - - def test_delete_reply_raises_forbidden(self, sqlite_session: Session) -> None: - comment = _comment() - _persist(sqlite_session, comment) - reply = WorkflowCommentReply(comment_id=comment.id, content="reply", created_by=OWNER_ID) - _persist(sqlite_session, reply) - - with pytest.raises(Forbidden): - WorkflowCommentService.delete_reply( - tenant_id=TENANT_ID, - app_id=APP_ID, - comment_id=comment.id, - reply_id=reply.id, - user_id=OUTSIDER_ID, - ) - - def test_delete_reply_raises_not_found(self, sqlite_session: Session) -> None: - with pytest.raises(NotFound): - WorkflowCommentService.delete_reply( - tenant_id=TENANT_ID, - app_id=APP_ID, - comment_id="missing-comment", - reply_id="missing-reply", - user_id=OWNER_ID, - ) - - def test_delete_reply_removes_mentions(self, sqlite_session: Session) -> None: - comment = _comment() - _persist(sqlite_session, comment) - reply = WorkflowCommentReply(comment_id=comment.id, content="reply", created_by=OWNER_ID) - _persist(sqlite_session, reply) - reply_id = reply.id - _persist( - sqlite_session, - WorkflowCommentMention(comment_id=comment.id, reply_id=reply.id, mentioned_user_id=USER_2_ID), - WorkflowCommentMention(comment_id=comment.id, reply_id=reply.id, mentioned_user_id=USER_3_ID), - ) - - WorkflowCommentService.delete_reply( - tenant_id=TENANT_ID, - app_id=APP_ID, - comment_id=comment.id, - reply_id=reply_id, - user_id=OWNER_ID, - ) - - sqlite_session.expire_all() - assert sqlite_session.get(WorkflowCommentReply, reply_id) is None - assert ( - sqlite_session.scalar( - select(func.count()) - .select_from(WorkflowCommentMention) - .where(WorkflowCommentMention.reply_id == reply_id) - ) - == 0 - ) - - def test_validate_comment_access_delegates_to_get_comment(self, sqlite_session: Session) -> None: - comment = _comment() - _persist(sqlite_session, comment) - - result = WorkflowCommentService.validate_comment_access(comment.id, TENANT_ID, APP_ID) - - assert result.id == comment.id - - def test_reply_lookup_is_scoped_to_tenant_app_and_comment(self, sqlite_session: Session) -> None: - comment = _comment() - other_comment = _comment(app_id=OTHER_APP_ID) - _persist(sqlite_session, comment, other_comment) - reply = WorkflowCommentReply(comment_id=comment.id, content="reply", created_by=OWNER_ID) - _persist(sqlite_session, reply) - - with pytest.raises(NotFound): - WorkflowCommentService.update_reply( - tenant_id=TENANT_ID, - app_id=OTHER_APP_ID, - comment_id=other_comment.id, - reply_id=reply.id, - user_id=OWNER_ID, - content="cross-thread update", - ) - - sqlite_session.refresh(reply) - assert reply.content == "reply"