From 8749d05ec140b1b46689e25099d594df87fbefd2 Mon Sep 17 00:00:00 2001 From: Heshan Wanigasooriya Date: Sun, 30 Aug 2026 07:39:03 -0400 Subject: [PATCH] feat(inbox): add draft replies with REST and MCP surfaces Replies to inbox messages could only be composed in the web view and were sent to the platform immediately, with no way to draft one for review or to create one programmatically. Give InboxReply a draft -> sent/failed lifecycle (migration 0002 backfills existing rows to "sent") and add apps/inbox/services.py as the single source of truth the web views, a new /api/v1/inbox REST router, and six new MCP tools all delegate to. Drafting is gated on use_inbox; delivering a reply is gated on reply_from_inbox. Message/reply lookups are scoped to the API key's workspace and account allowlist. The web composer now shows pending drafts with Send / Discard controls. Co-Authored-By: Claude Sonnet 5 --- README.md | 14 +- apps/api/api.py | 14 + apps/api/routers/inbox.py | 222 ++++++++++ apps/api/schemas.py | 116 ++++++ apps/api/tests/test_inbox_router.py | 291 ++++++++++++++ .../0002_inboxreply_draft_lifecycle.py | 75 ++++ apps/inbox/models.py | 22 +- apps/inbox/services.py | 209 ++++++++++ apps/inbox/tests/test_draft_reply_views.py | 145 +++++++ apps/inbox/tests/test_migration_0002.py | 65 +++ apps/inbox/tests/test_send_reply.py | 45 ++- apps/inbox/tests/test_services.py | 163 ++++++++ apps/inbox/urls.py | 4 + apps/inbox/views.py | 174 ++++---- apps/mcp/handlers.py | 380 ++++++++++++++++++ apps/mcp/tests/test_inbox_tools.py | 264 ++++++++++++ apps/mcp/tests/test_rest_parity.py | 73 ++++ pyproject.toml | 1 + .../inbox/partials/_draft_reply_item.html | 33 ++ templates/inbox/partials/_reply_composer.html | 32 +- 20 files changed, 2225 insertions(+), 117 deletions(-) create mode 100644 apps/api/routers/inbox.py create mode 100644 apps/api/tests/test_inbox_router.py create mode 100644 apps/inbox/migrations/0002_inboxreply_draft_lifecycle.py create mode 100644 apps/inbox/services.py create mode 100644 apps/inbox/tests/test_draft_reply_views.py create mode 100644 apps/inbox/tests/test_migration_0002.py create mode 100644 apps/inbox/tests/test_services.py create mode 100644 apps/mcp/tests/test_inbox_tools.py create mode 100644 templates/inbox/partials/_draft_reply_item.html diff --git a/README.md b/README.md index 6aac825f..a241e0ec 100644 --- a/README.md +++ b/README.md @@ -626,7 +626,7 @@ Issue an API key from **Organization → API Keys**. Keys are workspace-scoped, Authorization: Bearer bb_studio_... ``` -Permission keys: `create_posts`, `publish_directly`, `upload_media`, `view_analytics`. Each endpoint requires the relevant permission; missing permissions return `403`. +Permission keys: `create_posts`, `publish_directly`, `upload_media`, `view_analytics`, `use_inbox`, `reply_from_inbox`. Each endpoint requires the relevant permission; missing permissions return `403`. ### Rate Limits @@ -654,6 +654,12 @@ Rate-limit responses (`429`) include `Retry-After`, `X-RateLimit-Limit`, and `X- | `POST` | `/media` | Upload a media file (multipart) | `upload_media` | | `GET` | `/media/{media_id}` | Retrieve a media asset | — | | `GET` | `/media` | List media assets (filter, paginate) | — | +| `GET` | `/inbox` | List inbox messages (filter by status/type/account, paginate) | `use_inbox` | +| `GET` | `/inbox/{message_id}` | Read one inbox message with its reply thread | `use_inbox` | +| `POST` | `/inbox/{message_id}/replies` | Draft a reply (set `send: true` to deliver it now) | `use_inbox` (+ `reply_from_inbox` to send) | +| `PATCH` | `/inbox/replies/{reply_id}` | Edit a draft reply | `use_inbox` | +| `POST` | `/inbox/replies/{reply_id}/send` | Deliver a draft reply to the platform | `reply_from_inbox` | +| `DELETE` | `/inbox/replies/{reply_id}` | Discard a draft reply | `use_inbox` | | `POST` | `/mcp` | JSON-RPC 2.0 endpoint for MCP clients | — | All write endpoints accept `idempotency_key` (or `Idempotency-Key` header) for safe retries. @@ -676,6 +682,12 @@ The MCP server lives at `POST {APP_URL}/api/v1/mcp` and speaks JSON-RPC 2.0 over | `upload_media` | Upload a small base64-encoded file (≤ 1 MB raw). For larger files, use REST `POST /media`. | `upload_media` | | `get_account_analytics` | Channel analytics over a rolling 7–90 day window | `view_analytics` | | `get_post_analytics` | Per-platform metrics for a single post (safe for polling drafts) | `view_analytics` | +| `list_inbox_messages` | List inbox items (comments, mentions, DMs, reviews) with their reply threads | `use_inbox` | +| `get_inbox_message` | Retrieve one inbox message and its reply thread | `use_inbox` | +| `create_reply_draft` | Draft a reply to an inbox message (saved, not sent) | `use_inbox` | +| `update_reply_draft` | Replace the body of a draft (or failed) reply | `use_inbox` | +| `discard_reply_draft` | Delete a draft (or failed) reply | `use_inbox` | +| `send_reply` | Deliver a reply (`reply_id`, or `message_id` + `body` to draft-and-send) | `reply_from_inbox` | ### Connecting an MCP client diff --git a/apps/api/api.py b/apps/api/api.py index f34d95e4..10c00e6c 100644 --- a/apps/api/api.py +++ b/apps/api/api.py @@ -20,6 +20,7 @@ from apps.api.auth import ApiKeyAuth, McpAuth from apps.api.routers.accounts import router as accounts_router from apps.api.routers.analytics import router as analytics_router +from apps.api.routers.inbox import router as inbox_router from apps.api.routers.me import router as me_router from apps.api.routers.media import router as media_router from apps.api.routers.posts import router as posts_router @@ -76,6 +77,7 @@ class NoncedSwagger(Swagger): api.add_router("/posts", posts_router) api.add_router("/media", media_router) api.add_router("/analytics", analytics_router) +api.add_router("/inbox", inbox_router) # MCP Streamable HTTP transport. Same audit + rate limits as REST, but a # wider auth class: ``McpAuth`` accepts both bb_studio_ keys AND OAuth 2.1 # access tokens (Claude Desktop's native connector flow). Mounted last so @@ -233,6 +235,18 @@ def _action_for_path(method: str, path: str, *, status_code: int) -> str: return f"media.upload.{status_code}" if method == "GET": return f"media.read.{status_code}" + if "/inbox/" in path or path.endswith("/inbox"): + if "/replies" in path or "/reply" in path: + if path.endswith("/send"): + return f"inbox.reply.send.{status_code}" + if method == "POST": + return f"inbox.reply.create.{status_code}" + if method == "PATCH": + return f"inbox.reply.update.{status_code}" + if method == "DELETE": + return f"inbox.reply.discard.{status_code}" + if method == "GET": + return f"inbox.read.{status_code}" if "/mcp" in path: return f"mcp.error.{status_code}" if "/accounts" in path: diff --git a/apps/api/routers/inbox.py b/apps/api/routers/inbox.py new file mode 100644 index 00000000..e2e2b821 --- /dev/null +++ b/apps/api/routers/inbox.py @@ -0,0 +1,222 @@ +"""``/api/v1/inbox/*`` — read inbox messages and draft / send replies. + +The inbox equivalent of :mod:`apps.api.routers.posts`: every route is a +thin adapter over :mod:`apps.inbox.services`, the single source of truth +shared with the HTMX views and the MCP inbox tools. Message and reply +lookups are scoped to the key's workspace **and** its account allowlist, +returning 404 (never 403) for anything outside it so a partial-scope key +can't probe foreign IDs. + +Permissions mirror the web inbox: ``use_inbox`` to read and to manage +drafts, ``reply_from_inbox`` to actually deliver a reply to the platform. +""" + +from __future__ import annotations + +import uuid + +from django.db.models import QuerySet +from django.http import Http404, HttpRequest +from django.shortcuts import get_object_or_404 +from ninja import Query, Router +from ninja.errors import HttpError + +from apps.api.limits import enforce_http_rate_limits +from apps.api.middleware import log_audit_entry +from apps.api.pagination import decode_offset_cursor, encode_offset_cursor +from apps.api.schemas import ( + CreateReplyRequest, + InboxMessageResponse, + InboxMessagesListResponse, + InboxReplyResponse, + UpdateReplyRequest, +) +from apps.inbox.models import InboxMessage, InboxReply +from apps.inbox.services import ( + ReplyStateError, + create_reply_draft, + discard_reply_draft, + send_reply_now, + update_reply_draft, +) + +router = Router(tags=["inbox"]) + +_LIMIT_DEFAULT = 50 +_LIMIT_MAX = 100 + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _require_perm(request: HttpRequest, key: str) -> None: + membership = getattr(request, "workspace_membership", None) + if membership is None or not membership.effective_permissions.get(key, False): + raise HttpError(403, f"Permission denied: {key}") + + +def _allowlisted_account_ids(request: HttpRequest) -> set[uuid.UUID]: + return {sa.id for sa in request.api_key.social_accounts.all()} # type: ignore[attr-defined] + + +def _visible_messages_qs(request: HttpRequest) -> QuerySet[InboxMessage]: + """Messages in the key's workspace whose account is in the allowlist.""" + return InboxMessage.objects.filter( + workspace_id=request.api_key.workspace_id, # type: ignore[attr-defined] + social_account_id__in=_allowlisted_account_ids(request), + ).select_related("social_account") + + +def _get_message(request: HttpRequest, message_id: uuid.UUID) -> InboxMessage: + return get_object_or_404(_visible_messages_qs(request), id=message_id) + + +def _get_reply(request: HttpRequest, reply_id: uuid.UUID) -> InboxReply: + reply = get_object_or_404( + InboxReply.objects.select_related("inbox_message", "inbox_message__social_account", "author"), + id=reply_id, + inbox_message__workspace_id=request.api_key.workspace_id, # type: ignore[attr-defined] + ) + if reply.inbox_message.social_account_id not in _allowlisted_account_ids(request): + raise Http404() + return reply + + +# --------------------------------------------------------------------------- +# Routes +# --------------------------------------------------------------------------- + + +@router.get("/", response=InboxMessagesListResponse, summary="List inbox messages") +def list_messages( + request, + status: str | None = Query(None), + message_type: str | None = Query(None), + social_account_id: uuid.UUID | None = Query(None), + limit: int = Query(_LIMIT_DEFAULT, ge=1, le=_LIMIT_MAX), + cursor: str | None = Query(None), +): + enforce_http_rate_limits(request, is_write=False) + _require_perm(request, "use_inbox") + + if status is not None and status not in InboxMessage.Status.values: + raise HttpError(422, f"status must be one of {', '.join(InboxMessage.Status.values)}") + if message_type is not None and message_type not in InboxMessage.MessageType.values: + raise HttpError(422, f"message_type must be one of {', '.join(InboxMessage.MessageType.values)}") + + try: + offset = decode_offset_cursor(cursor) + except ValueError as exc: + raise HttpError(422, "cursor is not a valid pagination cursor") from exc + + qs = _visible_messages_qs(request).prefetch_related("replies__author") + if status: + qs = qs.filter(status=status) + if message_type: + qs = qs.filter(message_type=message_type) + if social_account_id is not None: + if social_account_id not in _allowlisted_account_ids(request): + raise HttpError(403, "social_account_id is not in this key's allowlist.") + qs = qs.filter(social_account_id=social_account_id) + qs = qs.order_by("-received_at", "id") + + rows = list(qs[offset : offset + limit + 1]) + has_more = len(rows) > limit + rows = rows[:limit] + log_audit_entry(request, action="inbox.list", target_id=None, status_code=200) + return InboxMessagesListResponse( + messages=[InboxMessageResponse.from_message(m, include_replies=True) for m in rows], + limit=limit, + next_cursor=encode_offset_cursor(offset + limit) if has_more else None, + ) + + +@router.get("/{message_id}", response=InboxMessageResponse, summary="Read one inbox message") +def retrieve_message(request, message_id: uuid.UUID): + enforce_http_rate_limits(request, is_write=False) + _require_perm(request, "use_inbox") + message = _get_message(request, message_id) + log_audit_entry(request, action="inbox.read", target_id=message.id, status_code=200) + return InboxMessageResponse.from_message(message, include_replies=True) + + +@router.post( + "/{message_id}/replies", + response={201: InboxReplyResponse}, + summary="Create a draft reply (optionally send it)", +) +def create_reply(request, message_id: uuid.UUID, payload: CreateReplyRequest): + enforce_http_rate_limits(request, is_write=True) + _require_perm(request, "use_inbox") + if payload.send: + _require_perm(request, "reply_from_inbox") + + message = _get_message(request, message_id) + try: + reply = create_reply_draft( + message=message, + body=payload.body, + author=request.user if not request.user.is_anonymous else None, + ) + except ValueError as exc: + raise HttpError(422, str(exc)) from exc + + if payload.send: + try: + send_reply_now(reply, actor=request.user if not request.user.is_anonymous else None) + except NotImplementedError: + pass # provider has no reply API; the local draft is recorded as sent + except ReplyStateError as exc: + raise HttpError(409, str(exc)) from exc + except Exception as exc: # platform refused it — reply is left in "failed" + raise HttpError(502, f"Reply not sent: {exc}") from exc + + log_audit_entry(request, action="inbox.reply.create", target_id=reply.id, status_code=201) + return 201, InboxReplyResponse.from_reply(reply) + + +@router.patch("/replies/{reply_id}", response=InboxReplyResponse, summary="Edit a draft reply") +def update_reply(request, reply_id: uuid.UUID, payload: UpdateReplyRequest): + enforce_http_rate_limits(request, is_write=True) + _require_perm(request, "use_inbox") + reply = _get_reply(request, reply_id) + try: + update_reply_draft(reply, body=payload.body) + except ReplyStateError as exc: + raise HttpError(409, str(exc)) from exc + except ValueError as exc: + raise HttpError(422, str(exc)) from exc + log_audit_entry(request, action="inbox.reply.update", target_id=reply.id, status_code=200) + return InboxReplyResponse.from_reply(reply) + + +@router.post("/replies/{reply_id}/send", response=InboxReplyResponse, summary="Send a draft reply") +def send_reply(request, reply_id: uuid.UUID): + enforce_http_rate_limits(request, is_write=True) + _require_perm(request, "reply_from_inbox") + reply = _get_reply(request, reply_id) + try: + send_reply_now(reply, actor=request.user if not request.user.is_anonymous else None) + except NotImplementedError: + pass + except ReplyStateError as exc: + raise HttpError(409, str(exc)) from exc + except Exception as exc: + raise HttpError(502, f"Reply not sent: {exc}") from exc + log_audit_entry(request, action="inbox.reply.send", target_id=reply.id, status_code=200) + return InboxReplyResponse.from_reply(reply) + + +@router.delete("/replies/{reply_id}", response={204: None}, summary="Discard a draft reply") +def delete_reply(request, reply_id: uuid.UUID): + enforce_http_rate_limits(request, is_write=True) + _require_perm(request, "use_inbox") + reply = _get_reply(request, reply_id) + try: + discard_reply_draft(reply) + except ReplyStateError as exc: + raise HttpError(409, str(exc)) from exc + log_audit_entry(request, action="inbox.reply.discard", target_id=reply_id, status_code=204) + return 204, None diff --git a/apps/api/schemas.py b/apps/api/schemas.py index 18a9834b..bdcbc78c 100644 --- a/apps/api/schemas.py +++ b/apps/api/schemas.py @@ -622,6 +622,122 @@ class PostAnalyticsResponse(Schema): platform_posts: list[PlatformPostAnalyticsResponse] +# --------------------------------------------------------------------------- +# /inbox — read + reply drafting +# --------------------------------------------------------------------------- + + +class InboxReplyResponse(Schema): + """One outbound reply to an inbox message. + + A reply is created as ``draft``, then a send step delivers it to the + platform and moves it to ``sent`` (or ``failed`` with a human-readable + ``send_error`` if the platform refused it). ``sent_at`` / + ``platform_reply_id`` are populated only once ``status == "sent"``. + """ + + id: uuid.UUID + inbox_message_id: uuid.UUID + status: str + body: str + author_email: str = "" + platform_reply_id: str = "" + send_error: str = "" + created_at: dt.datetime + updated_at: dt.datetime + sent_at: dt.datetime | None = None + + @field_serializer("created_at", "updated_at", "sent_at") + def _serialize_dt(self, value: dt.datetime | None) -> str | None: + return _serialize_utc_z(value) + + @classmethod + def from_reply(cls, reply) -> InboxReplyResponse: + author = getattr(reply, "author", None) + return cls( + id=reply.id, + inbox_message_id=reply.inbox_message_id, + status=reply.status, + body=reply.body, + author_email=(getattr(author, "email", "") or ""), + platform_reply_id=reply.platform_reply_id or "", + send_error=reply.send_error or "", + created_at=reply.created_at, + updated_at=reply.updated_at, + sent_at=reply.sent_at, + ) + + +class InboxMessageResponse(Schema): + """An inbound comment / mention / DM / review in the unified inbox.""" + + id: uuid.UUID + workspace_id: uuid.UUID + social_account_id: uuid.UUID + platform: str + message_type: str + status: str + sentiment: str + sender_name: str + sender_handle: str = "" + body: str + related_post_id: uuid.UUID | None = None + received_at: dt.datetime + created_at: dt.datetime + replies: list[InboxReplyResponse] = Field(default_factory=list) + + @field_serializer("received_at", "created_at") + def _serialize_dt(self, value: dt.datetime | None) -> str | None: + return _serialize_utc_z(value) + + @classmethod + def from_message(cls, message, *, include_replies: bool = False) -> InboxMessageResponse: + replies: list[InboxReplyResponse] = [] + if include_replies: + if "replies" in getattr(message, "_prefetched_objects_cache", {}): + rows = message.replies.all() + else: + rows = message.replies.select_related("author") + replies = [InboxReplyResponse.from_reply(r) for r in rows] + return cls( + id=message.id, + workspace_id=message.workspace_id, + social_account_id=message.social_account_id, + platform=message.social_account.platform, + message_type=message.message_type, + status=message.status, + sentiment=message.sentiment, + sender_name=message.sender_name, + sender_handle=message.sender_handle or "", + body=message.body or "", + related_post_id=message.related_post_id, + received_at=message.received_at, + created_at=message.created_at, + replies=replies, + ) + + +class InboxMessagesListResponse(Schema): + messages: list[InboxMessageResponse] + limit: int + next_cursor: str | None = None + + +class CreateReplyRequest(Schema): + body: str = Field(..., min_length=1, max_length=10_000, description="The reply text.") + send: bool = Field( + False, + description=( + "When true, immediately deliver the reply to the platform instead of " + "leaving it as a draft. Requires the ``reply_from_inbox`` permission." + ), + ) + + +class UpdateReplyRequest(Schema): + body: str = Field(..., min_length=1, max_length=10_000, description="Replacement reply text.") + + # --------------------------------------------------------------------------- # Error envelope (used by the exception handler in api.py) # --------------------------------------------------------------------------- diff --git a/apps/api/tests/test_inbox_router.py b/apps/api/tests/test_inbox_router.py new file mode 100644 index 00000000..a2e0869c --- /dev/null +++ b/apps/api/tests/test_inbox_router.py @@ -0,0 +1,291 @@ +"""``/api/v1/inbox/*`` — list messages, draft / send / discard replies.""" + +from __future__ import annotations + +import json +from datetime import timedelta + +import pytest +from django.test import Client +from django.utils import timezone + +from apps.api_keys import services +from apps.inbox.models import InboxMessage, InboxReply +from apps.members.models import PERMISSION_KEYS, OrgMembership, WorkspaceMembership + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture +def user(db): + from apps.accounts.models import User + + return User.objects.create_user( + email="inbox-agent@example.com", + password="testpass123", + name="Inbox Agent", + tos_accepted_at=timezone.now(), + ) + + +@pytest.fixture +def organization(db): + from apps.organizations.models import Organization + + return Organization.objects.create(name="Inbox Org") + + +@pytest.fixture +def workspace(db, organization): + from apps.workspaces.models import Workspace + + return Workspace.objects.create(name="Inbox WS", organization=organization) + + +@pytest.fixture +def owner_memberships(db, user, organization, workspace): + OrgMembership.objects.create(user=user, organization=organization, org_role=OrgMembership.OrgRole.OWNER) + return WorkspaceMembership.objects.create( + user=user, workspace=workspace, workspace_role=WorkspaceMembership.WorkspaceRole.OWNER + ) + + +@pytest.fixture +def account(db, workspace): + from apps.social_accounts.models import SocialAccount + + return SocialAccount.objects.create( + workspace=workspace, + platform="facebook", + account_platform_id="page-1", + account_name="Page", + connection_status="connected", + oauth_access_token="tok", + ) + + +@pytest.fixture +def other_account(db, workspace): + """A second account in the same workspace, NOT in the key's allowlist.""" + from apps.social_accounts.models import SocialAccount + + return SocialAccount.objects.create( + workspace=workspace, + platform="facebook", + account_platform_id="page-2", + account_name="Page 2", + connection_status="connected", + oauth_access_token="tok2", + ) + + +def _message(account, **kw): + defaults = dict( + workspace=account.workspace, + social_account=account, + platform_message_id="pm-1", + message_type=InboxMessage.MessageType.COMMENT, + sender_name="Ada", + sender_handle="ada", + body="hi?", + received_at=timezone.now() - timedelta(hours=1), + ) + defaults.update(kw) + return InboxMessage.objects.create(**defaults) + + +@pytest.fixture +def message(db, account): + return _message(account) + + +@pytest.fixture +def full_key(db, user, owner_memberships, workspace, account): + return services.issue_api_key( + workspace=workspace, + social_accounts=[account], + issued_by=user, + name="full", + permissions=list(PERMISSION_KEYS), + ) + + +@pytest.fixture +def draft_only_key(db, user, owner_memberships, workspace, account): + return services.issue_api_key( + workspace=workspace, + social_accounts=[account], + issued_by=user, + name="draft-only", + permissions=["use_inbox"], + ) + + +class _SecureClient(Client): + def generic(self, method, path, *args, **kwargs): + kwargs["secure"] = True + return super().generic(method, path, *args, **kwargs) + + +@pytest.fixture +def api(full_key): + return _SecureClient(HTTP_AUTHORIZATION=f"Bearer {full_key.plaintext_token}") + + +@pytest.fixture +def draft_api(draft_only_key): + return _SecureClient(HTTP_AUTHORIZATION=f"Bearer {draft_only_key.plaintext_token}") + + +# --------------------------------------------------------------------------- +# List + read +# --------------------------------------------------------------------------- + + +@pytest.mark.django_db +class TestListAndRead: + def test_list_returns_allowlisted_messages_with_replies(self, api, message): + InboxReply.objects.create(inbox_message=message, body="draft one") + r = api.get("/api/v1/inbox/") + assert r.status_code == 200, r.content + body = r.json() + assert len(body["messages"]) == 1 + msg = body["messages"][0] + assert msg["id"] == str(message.id) + assert msg["replies"][0]["body"] == "draft one" + assert msg["replies"][0]["status"] == "draft" + + def test_list_hides_messages_on_non_allowlisted_account(self, api, message, other_account): + _message(other_account, platform_message_id="pm-other") + r = api.get("/api/v1/inbox/") + ids = {m["id"] for m in r.json()["messages"]} + assert ids == {str(message.id)} + + def test_list_status_filter_validates(self, api, message): + r = api.get("/api/v1/inbox/?status=bogus") + assert r.status_code == 422 + + def test_retrieve_foreign_account_message_is_404(self, api, other_account): + m = _message(other_account, platform_message_id="pm-other") + r = api.get(f"/api/v1/inbox/{m.id}") + assert r.status_code == 404 + + def test_list_requires_use_inbox(self, message, user, owner_memberships, workspace, account): + key = services.issue_api_key( + workspace=workspace, + social_accounts=[account], + issued_by=user, + name="noperm", + permissions=["view_analytics"], + ) + c = _SecureClient(HTTP_AUTHORIZATION=f"Bearer {key.plaintext_token}") + r = c.get("/api/v1/inbox/") + assert r.status_code == 403 + + +# --------------------------------------------------------------------------- +# Draft lifecycle +# --------------------------------------------------------------------------- + + +@pytest.mark.django_db +class TestReplyDrafts: + def test_create_draft(self, api, message): + r = api.post( + f"/api/v1/inbox/{message.id}/replies", + data=json.dumps({"body": "drafted via API"}), + content_type="application/json", + ) + assert r.status_code == 201, r.content + body = r.json() + assert body["status"] == "draft" + assert body["body"] == "drafted via API" + assert InboxReply.objects.get(id=body["id"]).author_id is not None + + def test_create_and_send(self, api, message): + from unittest.mock import patch + + with patch("apps.inbox.services._dispatch_to_platform", return_value="plat-1"): + r = api.post( + f"/api/v1/inbox/{message.id}/replies", + data=json.dumps({"body": "send now", "send": True}), + content_type="application/json", + ) + assert r.status_code == 201, r.content + assert r.json()["status"] == "sent" + assert r.json()["platform_reply_id"] == "plat-1" + + def test_draft_only_key_cannot_send(self, draft_api, message): + r = draft_api.post( + f"/api/v1/inbox/{message.id}/replies", + data=json.dumps({"body": "x", "send": True}), + content_type="application/json", + ) + assert r.status_code == 403 + assert InboxReply.objects.count() == 0 + + def test_draft_only_key_can_draft(self, draft_api, message): + r = draft_api.post( + f"/api/v1/inbox/{message.id}/replies", + data=json.dumps({"body": "just a draft"}), + content_type="application/json", + ) + assert r.status_code == 201 + + def test_patch_draft_body(self, api, message): + reply = InboxReply.objects.create(inbox_message=message, body="v1") + r = api.patch( + f"/api/v1/inbox/replies/{reply.id}", + data=json.dumps({"body": "v2"}), + content_type="application/json", + ) + assert r.status_code == 200 + reply.refresh_from_db() + assert reply.body == "v2" + + def test_patch_sent_reply_conflicts(self, api, message): + reply = InboxReply.objects.create(inbox_message=message, body="v1", status=InboxReply.Status.SENT) + r = api.patch( + f"/api/v1/inbox/replies/{reply.id}", + data=json.dumps({"body": "v2"}), + content_type="application/json", + ) + assert r.status_code == 409 + + def test_send_endpoint_delivers(self, api, message): + from unittest.mock import patch + + reply = InboxReply.objects.create(inbox_message=message, body="ready") + with patch("apps.inbox.services._dispatch_to_platform", return_value="plat-7"): + r = api.post(f"/api/v1/inbox/replies/{reply.id}/send") + assert r.status_code == 200 + assert r.json()["status"] == "sent" + + def test_send_endpoint_platform_failure_is_502(self, api, message): + from unittest.mock import patch + + reply = InboxReply.objects.create(inbox_message=message, body="ready") + with patch("apps.inbox.services._dispatch_to_platform", side_effect=RuntimeError("no")): + r = api.post(f"/api/v1/inbox/replies/{reply.id}/send") + assert r.status_code == 502 + reply.refresh_from_db() + assert reply.status == InboxReply.Status.FAILED + + def test_delete_draft(self, api, message): + reply = InboxReply.objects.create(inbox_message=message, body="scrap") + r = api.delete(f"/api/v1/inbox/replies/{reply.id}") + assert r.status_code == 204 + assert not InboxReply.objects.filter(pk=reply.pk).exists() + + def test_delete_sent_reply_conflicts(self, api, message): + reply = InboxReply.objects.create(inbox_message=message, body="done", status=InboxReply.Status.SENT) + r = api.delete(f"/api/v1/inbox/replies/{reply.id}") + assert r.status_code == 409 + + def test_reply_on_foreign_account_message_is_404(self, api, other_account): + m = _message(other_account, platform_message_id="pm-other") + reply = InboxReply.objects.create(inbox_message=m, body="x") + r = api.delete(f"/api/v1/inbox/replies/{reply.id}") + assert r.status_code == 404 diff --git a/apps/inbox/migrations/0002_inboxreply_draft_lifecycle.py b/apps/inbox/migrations/0002_inboxreply_draft_lifecycle.py new file mode 100644 index 00000000..fd783258 --- /dev/null +++ b/apps/inbox/migrations/0002_inboxreply_draft_lifecycle.py @@ -0,0 +1,75 @@ +"""Give ``InboxReply`` a draft → sent/failed lifecycle. + +Before this, an ``InboxReply`` row was written only *after* the platform +accepted the reply, so ``sent_at`` could be ``auto_now_add`` and every +row implicitly meant "delivered". Draft replies (created by an agent, or +saved from the composer for later) need a row that exists before any +send, so we add an explicit ``status`` plus ``created_at`` / ``updated_at`` +and make ``sent_at`` nullable. Every pre-existing row is a delivered +reply, so it is backfilled to ``sent``. +""" + +import django.utils.timezone +from django.db import migrations, models + + +def _mark_existing_sent(apps, schema_editor): + InboxReply = apps.get_model("inbox", "InboxReply") + InboxReply.objects.all().update(status="sent") + # ``created_at`` got a flat default at column-add time; line it up with + # the real send time where we have one so ordering stays sensible. + for reply in InboxReply.objects.exclude(sent_at=None).iterator(): + InboxReply.objects.filter(pk=reply.pk).update(created_at=reply.sent_at) + + +def _noop(apps, schema_editor): + pass + + +class Migration(migrations.Migration): + dependencies = [ + ("inbox", "0001_initial"), + ] + + operations = [ + migrations.AddField( + model_name="inboxreply", + name="status", + field=models.CharField( + choices=[("draft", "Draft"), ("sent", "Sent"), ("failed", "Failed")], + db_index=True, + default="draft", + max_length=10, + ), + ), + migrations.AddField( + model_name="inboxreply", + name="send_error", + field=models.TextField(blank=True, default=""), + ), + migrations.AddField( + model_name="inboxreply", + name="created_at", + field=models.DateTimeField( + auto_now_add=True, + default=django.utils.timezone.now, + ), + preserve_default=False, + ), + migrations.AddField( + model_name="inboxreply", + name="updated_at", + field=models.DateTimeField(auto_now=True, default=django.utils.timezone.now), + preserve_default=False, + ), + migrations.AlterField( + model_name="inboxreply", + name="sent_at", + field=models.DateTimeField(blank=True, null=True), + ), + migrations.AlterModelOptions( + name="inboxreply", + options={"ordering": ["created_at"]}, + ), + migrations.RunPython(_mark_existing_sent, _noop), + ] diff --git a/apps/inbox/models.py b/apps/inbox/models.py index d687cfd6..2e7cee79 100644 --- a/apps/inbox/models.py +++ b/apps/inbox/models.py @@ -124,6 +124,11 @@ def platform(self): class InboxReply(models.Model): + class Status(models.TextChoices): + DRAFT = "draft", "Draft" + SENT = "sent", "Sent" + FAILED = "failed", "Failed" + id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) inbox_message = models.ForeignKey( InboxMessage, @@ -137,15 +142,26 @@ class InboxReply(models.Model): related_name="inbox_replies", ) body = models.TextField() + status = models.CharField( + max_length=10, + choices=Status.choices, + default=Status.DRAFT, + db_index=True, + ) platform_reply_id = models.CharField(max_length=255, blank=True, default="") - sent_at = models.DateTimeField(auto_now_add=True) + send_error = models.TextField(blank=True, default="") + # ``sent_at`` is null until the reply is actually delivered to the platform; + # a row now exists in ``draft``/``failed`` states before any send happens. + sent_at = models.DateTimeField(null=True, blank=True) + created_at = models.DateTimeField(auto_now_add=True) + updated_at = models.DateTimeField(auto_now=True) class Meta: db_table = "inbox_reply" - ordering = ["sent_at"] + ordering = ["created_at"] def __str__(self): - return f"Reply by {self.author} on {self.sent_at:%Y-%m-%d %H:%M}" + return f"{self.get_status_display()} reply by {self.author} ({self.created_at:%Y-%m-%d %H:%M})" class InternalNote(models.Model): diff --git a/apps/inbox/services.py b/apps/inbox/services.py new file mode 100644 index 00000000..9bf5e425 --- /dev/null +++ b/apps/inbox/services.py @@ -0,0 +1,209 @@ +"""Service layer for the Unified Social Inbox (F-3.1). + +Both the HTMX views and the programmatic surfaces (the ``/api/v1/inbox`` +REST router and the MCP inbox tools) go through these functions so the +three can't drift — the same rule the composer follows with +``apps.composer.services``. + +A reply now has a lifecycle: it is created as a ``draft``, then a separate +send step delivers it to the platform and moves it to ``sent`` (or +``failed`` if the platform refused it). The platform-dispatch logic used +to live in ``apps/inbox/views.py``; it moved here verbatim. +""" + +from __future__ import annotations + +import logging +from datetime import timedelta + +from django.db import transaction +from django.utils import timezone + +from providers import get_provider + +from .models import InboxMessage, InboxReply, InboxSLAConfig + +logger = logging.getLogger(__name__) + +# Message types answered on a comment edge rather than a messaging endpoint. +_COMMENT_LIKE_TYPES = { + InboxMessage.MessageType.COMMENT, + InboxMessage.MessageType.MENTION, + InboxMessage.MessageType.REVIEW, +} + +# Past this age Meta only accepts a reply tagged as written by a person. +HUMAN_AGENT_AFTER = timedelta(hours=24) + +# States a reply can be sent (or re-sent) from. +_SENDABLE_STATUSES = {InboxReply.Status.DRAFT, InboxReply.Status.FAILED} + + +class ReplyStateError(ValueError): + """Raised when an operation is not valid for a reply's current status.""" + + +# --------------------------------------------------------------------------- +# Platform dispatch (moved from views.py, behaviour unchanged) +# --------------------------------------------------------------------------- + + +def _reply_failure_reason(exc: Exception) -> str: + """A short, actionable reason for the user. + + The platform's own error text carries internal diagnostics (trace IDs, + raw API JSON) that mean nothing to a workspace member, so it stays in + the log and the UI/API gets a stable sentence instead. + """ + from providers.exceptions import OAuthError, RateLimitError, TokenExpiredError + + if isinstance(exc, RateLimitError): + return "the account has hit its rate limit. Wait a few minutes and try again." + if isinstance(exc, TokenExpiredError | OAuthError): + return "the connection has expired. Reconnect the account in Workspace Settings." + return "the platform rejected it. Try again, or reconnect the account if this keeps happening." + + +def _dispatch_to_platform(message: InboxMessage, body: str) -> str: + """Post ``body`` back to the platform and return the platform's reply ID. + + Raises if the platform refuses it, so the caller can avoid recording a + reply as delivered when it never was. + """ + from apps.publisher.engine import _resolve_publish_credentials + + account = message.social_account + provider = get_provider(account.platform, _resolve_publish_credentials(account)) + + # The messaging endpoints address a person, not a message, so carry the + # sender's platform-scoped ID alongside the original payload. + extra = dict(message.extra or {}) + if message.sender_handle: + extra.setdefault("recipient_id", message.sender_handle) + + if message.message_type in _COMMENT_LIKE_TYPES: + result = provider.reply_to_comment( + access_token=account.oauth_access_token, + comment_id=message.platform_message_id, + text=body, + extra=extra, + ) + else: + overdue = timezone.now() - message.received_at > HUMAN_AGENT_AFTER + result = provider.reply_to_message( + access_token=account.oauth_access_token, + message_id=message.platform_message_id, + text=body, + extra=extra, + human_agent=overdue, + ) + + return result.platform_message_id + + +def _apply_post_send_side_effects(message: InboxMessage) -> None: + """Resolve or open the message after a reply goes out, per SLA config.""" + sla_config = InboxSLAConfig.objects.filter(workspace=message.workspace, is_active=True).first() + if sla_config and sla_config.auto_resolve_on_reply: + if message.status != InboxMessage.Status.RESOLVED: + message.status = InboxMessage.Status.RESOLVED + message.save(update_fields=["status"]) + elif message.status == InboxMessage.Status.UNREAD: + message.status = InboxMessage.Status.OPEN + message.save(update_fields=["status"]) + + +# --------------------------------------------------------------------------- +# Draft lifecycle +# --------------------------------------------------------------------------- + + +def create_reply_draft(*, message: InboxMessage, body: str, author=None) -> InboxReply: + """Create a ``draft`` reply against ``message``. Not sent anywhere.""" + body = (body or "").strip() + if not body: + raise ValueError("Reply body cannot be empty.") + return InboxReply.objects.create( + inbox_message=message, + author=author, + body=body, + status=InboxReply.Status.DRAFT, + ) + + +def update_reply_draft(reply: InboxReply, *, body: str) -> InboxReply: + """Edit a draft (or failed) reply's body.""" + if reply.status not in _SENDABLE_STATUSES: + raise ReplyStateError(f"A {reply.get_status_display().lower()} reply cannot be edited.") + body = (body or "").strip() + if not body: + raise ValueError("Reply body cannot be empty.") + reply.body = body + reply.save(update_fields=["body", "updated_at"]) + return reply + + +def discard_reply_draft(reply: InboxReply) -> None: + """Delete a draft (or failed) reply. Sent replies are permanent.""" + if reply.status not in _SENDABLE_STATUSES: + raise ReplyStateError(f"A {reply.get_status_display().lower()} reply cannot be discarded.") + reply.delete() + + +def send_reply_now(reply: InboxReply, *, actor=None) -> InboxReply: + """Deliver an existing draft/failed reply to the platform. + + On a platform refusal the row is kept and moved to ``failed`` with a + human-readable ``send_error`` so the team can retry; the underlying + exception is re-raised for the caller to shape into its own error. + ``NotImplementedError`` (provider has no reply API) is not a failure — + the reply is recorded locally with an empty ``platform_reply_id``, + matching the pre-existing behaviour. + """ + if reply.status not in _SENDABLE_STATUSES: + raise ReplyStateError(f"A {reply.get_status_display().lower()} reply cannot be sent again.") + + message = reply.inbox_message + if actor is not None and reply.author_id is None: + reply.author = actor + + try: + platform_reply_id = _dispatch_to_platform(message, reply.body) + except NotImplementedError: + logger.info( + "Provider %s cannot send replies; recording reply %s locally.", + message.social_account.platform, + reply.id, + ) + platform_reply_id = "" + except Exception as exc: + logger.exception("Failed to send inbox reply %s (%s)", reply.id, message.social_account.platform) + reply.status = InboxReply.Status.FAILED + reply.send_error = _reply_failure_reason(exc) + reply.save(update_fields=["status", "send_error", "author", "updated_at"]) + raise + + reply.status = InboxReply.Status.SENT + reply.platform_reply_id = platform_reply_id + reply.send_error = "" + reply.sent_at = timezone.now() + reply.save(update_fields=["status", "platform_reply_id", "send_error", "sent_at", "author", "updated_at"]) + + _apply_post_send_side_effects(message) + return reply + + +def send_reply(*, message: InboxMessage, body: str, author=None) -> InboxReply: + """Create a reply and send it in one step (the classic composer flow). + + If the platform refuses it, the ``failed`` row is removed and the + exception propagates — the thread must never show a reply the customer + never received. ``NotImplementedError`` keeps the local record. + """ + with transaction.atomic(): + reply = create_reply_draft(message=message, body=body, author=author) + try: + return send_reply_now(reply, actor=author) + except Exception: + InboxReply.objects.filter(pk=reply.pk, status=InboxReply.Status.FAILED).delete() + raise diff --git a/apps/inbox/tests/test_draft_reply_views.py b/apps/inbox/tests/test_draft_reply_views.py new file mode 100644 index 00000000..573c4286 --- /dev/null +++ b/apps/inbox/tests/test_draft_reply_views.py @@ -0,0 +1,145 @@ +"""HTMX views for drafting, sending and discarding inbox replies.""" + +from datetime import timedelta +from unittest.mock import patch + +import pytest +from django.utils import timezone + +from apps.inbox.models import InboxMessage, InboxReply +from apps.members.models import WorkspaceMembership +from apps.social_accounts.models import SocialAccount + + +@pytest.fixture +def workspace(db, organization): + from apps.workspaces.models import Workspace + + return Workspace.objects.create(name="Draft WS", organization=organization) + + +@pytest.fixture +def account(db, workspace): + return SocialAccount.objects.create( + workspace=workspace, + platform="facebook", + account_platform_id="page-1", + account_name="Page", + oauth_access_token="tok", + ) + + +@pytest.fixture +def message(db, account): + return InboxMessage.objects.create( + workspace=account.workspace, + social_account=account, + platform_message_id="pm-1", + message_type=InboxMessage.MessageType.COMMENT, + sender_name="Ada", + sender_handle="ada", + body="hi?", + received_at=timezone.now() - timedelta(hours=1), + ) + + +def _member(workspace, user, role): + return WorkspaceMembership.objects.create(user=user, workspace=workspace, workspace_role=role) + + +def _url(workspace, path): + return f"/workspace/{workspace.id}/inbox/{path}" + + +@pytest.mark.django_db +def test_save_reply_draft_creates_draft(client, workspace, account, message, org_owner, user): + _member(workspace, user, WorkspaceMembership.WorkspaceRole.OWNER) + client.force_login(user) + + resp = client.post(_url(workspace, f"{message.id}/reply/draft/"), {"body": "draft answer"}) + + assert resp.status_code == 200 + reply = InboxReply.objects.get(inbox_message=message) + assert reply.status == InboxReply.Status.DRAFT + assert reply.body == "draft answer" + assert reply.author == user + # The refreshed panel shows the pending draft. + assert b"draft answer" in resp.content + + +@pytest.mark.django_db +def test_save_reply_draft_denied_for_viewer(client, workspace, account, message, org_owner, user): + _member(workspace, user, WorkspaceMembership.WorkspaceRole.VIEWER) + client.force_login(user) + + resp = client.post(_url(workspace, f"{message.id}/reply/draft/"), {"body": "nope"}) + + assert resp.status_code == 403 + assert InboxReply.objects.count() == 0 + + +@pytest.mark.django_db +def test_send_reply_draft_delivers(client, workspace, account, message, org_owner, user): + _member(workspace, user, WorkspaceMembership.WorkspaceRole.OWNER) + client.force_login(user) + reply = InboxReply.objects.create(inbox_message=message, author=user, body="ready") + + with patch("apps.inbox.services._dispatch_to_platform", return_value="plat-9"): + resp = client.post(_url(workspace, f"replies/{reply.id}/send/")) + + assert resp.status_code == 200 + reply.refresh_from_db() + assert reply.status == InboxReply.Status.SENT + assert reply.platform_reply_id == "plat-9" + + +@pytest.mark.django_db +def test_send_reply_draft_failure_keeps_failed_row(client, workspace, account, message, org_owner, user): + _member(workspace, user, WorkspaceMembership.WorkspaceRole.OWNER) + client.force_login(user) + reply = InboxReply.objects.create(inbox_message=message, author=user, body="ready") + + with patch("apps.inbox.services._dispatch_to_platform", side_effect=RuntimeError("no")): + resp = client.post(_url(workspace, f"replies/{reply.id}/send/")) + + assert resp.status_code == 200 + assert resp["HX-Reply-Failed"] == "1" + reply.refresh_from_db() + assert reply.status == InboxReply.Status.FAILED + + +@pytest.mark.django_db +def test_send_reply_draft_denied_for_viewer(client, workspace, account, message, org_owner, user): + _member(workspace, user, WorkspaceMembership.WorkspaceRole.VIEWER) + client.force_login(user) + reply = InboxReply.objects.create(inbox_message=message, body="ready") + + resp = client.post(_url(workspace, f"replies/{reply.id}/send/")) + + assert resp.status_code == 403 + reply.refresh_from_db() + assert reply.status == InboxReply.Status.DRAFT + + +@pytest.mark.django_db +def test_discard_reply_draft_removes_it(client, workspace, account, message, org_owner, user): + _member(workspace, user, WorkspaceMembership.WorkspaceRole.OWNER) + client.force_login(user) + reply = InboxReply.objects.create(inbox_message=message, author=user, body="scrap") + + resp = client.post(_url(workspace, f"replies/{reply.id}/discard/")) + + assert resp.status_code == 200 + assert not InboxReply.objects.filter(pk=reply.pk).exists() + + +@pytest.mark.django_db +def test_discard_rejects_sent_reply(client, workspace, account, message, org_owner, user): + _member(workspace, user, WorkspaceMembership.WorkspaceRole.OWNER) + client.force_login(user) + reply = InboxReply.objects.create(inbox_message=message, author=user, body="done", status=InboxReply.Status.SENT) + + resp = client.post(_url(workspace, f"replies/{reply.id}/discard/")) + + assert resp.status_code == 409 + assert InboxReply.objects.filter(pk=reply.pk).exists() diff --git a/apps/inbox/tests/test_migration_0002.py b/apps/inbox/tests/test_migration_0002.py new file mode 100644 index 00000000..ef9442b8 --- /dev/null +++ b/apps/inbox/tests/test_migration_0002.py @@ -0,0 +1,65 @@ +"""Regression for the ``0002_inboxreply_draft_lifecycle`` data migration. + +Before 0002 an ``InboxReply`` row existed only once a send had succeeded, +so the backfill must mark every pre-existing row ``sent`` (not the new +``draft`` default) and line its ``created_at`` up with the real send time. +""" + +from __future__ import annotations + +import importlib +from datetime import timedelta + +import pytest +from django.utils import timezone + +from apps.inbox.models import InboxMessage, InboxReply +from apps.social_accounts.models import SocialAccount + +migration_module = importlib.import_module("apps.inbox.migrations.0002_inboxreply_draft_lifecycle") + + +@pytest.fixture +def workspace(db, organization): + from apps.workspaces.models import Workspace + + return Workspace.objects.create(name="Mig WS", organization=organization) + + +@pytest.fixture +def message(db, workspace): + account = SocialAccount.objects.create( + workspace=workspace, + platform="facebook", + account_platform_id="page-1", + account_name="Page", + oauth_access_token="tok", + ) + return InboxMessage.objects.create( + workspace=workspace, + social_account=account, + platform_message_id="pm-1", + message_type=InboxMessage.MessageType.COMMENT, + sender_name="Ada", + body="hi?", + received_at=timezone.now() - timedelta(hours=2), + ) + + +@pytest.mark.django_db +def test_backfill_marks_existing_replies_sent(message): + from django.apps import apps as global_apps + + sent_at = timezone.now() - timedelta(hours=1) + reply = InboxReply.objects.create(inbox_message=message, body="delivered") + # Simulate a pre-0002 row: it predates the status column and was only + # ever written post-send. + InboxReply.objects.filter(pk=reply.pk).update( + status=InboxReply.Status.DRAFT, sent_at=sent_at, created_at=timezone.now() + ) + + migration_module._mark_existing_sent(global_apps, None) + + reply.refresh_from_db() + assert reply.status == InboxReply.Status.SENT + assert reply.created_at == sent_at diff --git a/apps/inbox/tests/test_send_reply.py b/apps/inbox/tests/test_send_reply.py index 22b0100d..de9e823a 100644 --- a/apps/inbox/tests/test_send_reply.py +++ b/apps/inbox/tests/test_send_reply.py @@ -3,6 +3,9 @@ Also covers the two behaviours that decide whether Meta accepts a reply at all: tagging a late reply as written by a human, and never recording a reply the platform refused. + +The platform-dispatch logic lives in ``apps.inbox.services`` now; the view is a +thin wrapper over it. """ from datetime import timedelta @@ -13,7 +16,7 @@ from django.utils import timezone from apps.inbox.models import InboxMessage, InboxReply -from apps.inbox.views import _send_platform_reply +from apps.inbox.services import _dispatch_to_platform from apps.social_accounts.models import SocialAccount @@ -62,8 +65,8 @@ def test_comment_goes_to_the_comment_edge(fb_account): message = _message(fb_account, message_type=InboxMessage.MessageType.COMMENT) provider = _provider() - with patch("apps.inbox.views.get_provider", return_value=provider): - assert _send_platform_reply(message, "Thanks!") == "c-1" + with patch("apps.inbox.services.get_provider", return_value=provider): + assert _dispatch_to_platform(message, "Thanks!") == "c-1" provider.reply_to_comment.assert_called_once() provider.reply_to_message.assert_not_called() @@ -74,8 +77,8 @@ def test_mention_is_treated_as_a_comment(fb_account): message = _message(fb_account, message_type=InboxMessage.MessageType.MENTION) provider = _provider() - with patch("apps.inbox.views.get_provider", return_value=provider): - _send_platform_reply(message, "Thanks for the shout-out") + with patch("apps.inbox.services.get_provider", return_value=provider): + _dispatch_to_platform(message, "Thanks for the shout-out") provider.reply_to_comment.assert_called_once() @@ -111,7 +114,7 @@ def test_a_provider_that_cannot_reply_records_the_reply_locally(client, fb_accou message = _message(fb_account, message_type=InboxMessage.MessageType.DM) client.force_login(user) - with patch("apps.inbox.views._send_platform_reply", side_effect=NotImplementedError): + with patch("apps.inbox.services._dispatch_to_platform", side_effect=NotImplementedError): response = client.post( f"/workspace/{fb_account.workspace_id}/inbox/{message.id}/reply/", {"body": "Noted internally"}, @@ -120,6 +123,7 @@ def test_a_provider_that_cannot_reply_records_the_reply_locally(client, fb_accou assert response.status_code == 200 assert "HX-Reply-Failed" not in response reply = InboxReply.objects.get(inbox_message=message) + assert reply.status == InboxReply.Status.SENT assert reply.platform_reply_id == "" @@ -135,7 +139,7 @@ def test_the_error_shown_to_users_carries_no_raw_api_text(client, fb_account, or client.force_login(user) raw = 'Facebook API error 401: {"error":{"fbtrace_id":"AFC8u7xsP__NwLs"}}' - with patch("apps.inbox.views._send_platform_reply", side_effect=APIError(raw, platform="facebook")): + with patch("apps.inbox.services._dispatch_to_platform", side_effect=APIError(raw, platform="facebook")): response = client.post( f"/workspace/{fb_account.workspace_id}/inbox/{message.id}/reply/", {"body": "This will fail"}, @@ -157,7 +161,10 @@ def test_an_expired_connection_gets_a_reconnect_hint(client, fb_account, org_own message = _message(fb_account, message_type=InboxMessage.MessageType.DM) client.force_login(user) - with patch("apps.inbox.views._send_platform_reply", side_effect=TokenExpiredError("expired", platform="facebook")): + with patch( + "apps.inbox.services._dispatch_to_platform", + side_effect=TokenExpiredError("expired", platform="facebook"), + ): response = client.post( f"/workspace/{fb_account.workspace_id}/inbox/{message.id}/reply/", {"body": "hi"}, @@ -170,8 +177,8 @@ def test_recent_dm_replies_without_the_human_agent_tag(fb_account): message = _message(fb_account, message_type=InboxMessage.MessageType.DM, hours_ago=2) provider = _provider() - with patch("apps.inbox.views.get_provider", return_value=provider): - _send_platform_reply(message, "On it") + with patch("apps.inbox.services.get_provider", return_value=provider): + _dispatch_to_platform(message, "On it") assert provider.reply_to_message.call_args.kwargs["human_agent"] is False @@ -180,8 +187,8 @@ def test_dm_older_than_24_hours_is_tagged_human_agent(fb_account): message = _message(fb_account, message_type=InboxMessage.MessageType.DM, hours_ago=30) provider = _provider() - with patch("apps.inbox.views.get_provider", return_value=provider): - _send_platform_reply(message, "Sorry for the delay") + with patch("apps.inbox.services.get_provider", return_value=provider): + _dispatch_to_platform(message, "Sorry for the delay") assert provider.reply_to_message.call_args.kwargs["human_agent"] is True @@ -190,8 +197,8 @@ def test_sender_handle_is_passed_as_the_recipient(fb_account): message = _message(fb_account, message_type=InboxMessage.MessageType.DM, sender_handle="psid-99") provider = _provider() - with patch("apps.inbox.views.get_provider", return_value=provider): - _send_platform_reply(message, "Hi") + with patch("apps.inbox.services.get_provider", return_value=provider): + _dispatch_to_platform(message, "Hi") assert provider.reply_to_message.call_args.kwargs["extra"]["recipient_id"] == "psid-99" @@ -205,8 +212,8 @@ def test_existing_extra_recipient_is_not_overwritten(fb_account): ) provider = _provider() - with patch("apps.inbox.views.get_provider", return_value=provider): - _send_platform_reply(message, "Hi") + with patch("apps.inbox.services.get_provider", return_value=provider): + _dispatch_to_platform(message, "Hi") assert provider.reply_to_message.call_args.kwargs["extra"]["recipient_id"] == "from-payload" @@ -221,7 +228,7 @@ def test_failed_send_records_no_reply(client, fb_account, org_owner, user): message = _message(fb_account, message_type=InboxMessage.MessageType.DM) client.force_login(user) - with patch("apps.inbox.views._send_platform_reply", side_effect=RuntimeError("Meta said no")): + with patch("apps.inbox.services._dispatch_to_platform", side_effect=RuntimeError("Meta said no")): response = client.post( f"/workspace/{fb_account.workspace_id}/inbox/{message.id}/reply/", {"body": "This will fail"}, @@ -245,7 +252,7 @@ def test_successful_send_records_the_reply(client, fb_account, org_owner, user): message = _message(fb_account, message_type=InboxMessage.MessageType.DM) client.force_login(user) - with patch("apps.inbox.views._send_platform_reply", return_value="mid.sent"): + with patch("apps.inbox.services._dispatch_to_platform", return_value="mid.sent"): response = client.post( f"/workspace/{fb_account.workspace_id}/inbox/{message.id}/reply/", {"body": "Happy to help"}, @@ -254,4 +261,6 @@ def test_successful_send_records_the_reply(client, fb_account, org_owner, user): assert response.status_code == 200 assert "HX-Reply-Failed" not in response reply = InboxReply.objects.get(inbox_message=message) + assert reply.status == InboxReply.Status.SENT + assert reply.sent_at is not None assert reply.platform_reply_id == "mid.sent" diff --git a/apps/inbox/tests/test_services.py b/apps/inbox/tests/test_services.py new file mode 100644 index 00000000..77c0561c --- /dev/null +++ b/apps/inbox/tests/test_services.py @@ -0,0 +1,163 @@ +"""Draft-reply lifecycle in ``apps.inbox.services``. + +Covers create / edit / discard / send, the failed-send retry path, and +the SLA auto-resolve side effect — independently of any HTTP surface. +""" + +from datetime import timedelta +from unittest.mock import patch + +import pytest +from django.utils import timezone + +from apps.inbox import services +from apps.inbox.models import InboxMessage, InboxReply, InboxSLAConfig +from apps.social_accounts.models import SocialAccount + + +@pytest.fixture +def workspace(db, organization): + from apps.workspaces.models import Workspace + + return Workspace.objects.create(name="Svc WS", organization=organization) + + +@pytest.fixture +def account(db, workspace): + return SocialAccount.objects.create( + workspace=workspace, + platform="facebook", + account_platform_id="page-1", + account_name="Page", + oauth_access_token="tok", + ) + + +@pytest.fixture +def message(db, account): + return InboxMessage.objects.create( + workspace=account.workspace, + social_account=account, + platform_message_id="pm-1", + message_type=InboxMessage.MessageType.COMMENT, + sender_name="Ada", + sender_handle="ada", + body="hi?", + received_at=timezone.now() - timedelta(hours=1), + ) + + +def test_create_reply_draft_starts_in_draft(message, user): + reply = services.create_reply_draft(message=message, body=" hello ", author=user) + assert reply.status == InboxReply.Status.DRAFT + assert reply.body == "hello" # trimmed + assert reply.sent_at is None + assert reply.author == user + + +def test_create_reply_draft_rejects_blank(message): + with pytest.raises(ValueError): + services.create_reply_draft(message=message, body=" ") + + +def test_update_reply_draft_changes_body(message): + reply = services.create_reply_draft(message=message, body="v1") + services.update_reply_draft(reply, body="v2") + reply.refresh_from_db() + assert reply.body == "v2" + + +def test_update_rejects_sent_reply(message): + reply = services.create_reply_draft(message=message, body="v1") + reply.status = InboxReply.Status.SENT + reply.save(update_fields=["status"]) + with pytest.raises(services.ReplyStateError): + services.update_reply_draft(reply, body="v2") + + +def test_discard_removes_draft(message): + reply = services.create_reply_draft(message=message, body="bye") + services.discard_reply_draft(reply) + assert not InboxReply.objects.filter(pk=reply.pk).exists() + + +def test_discard_rejects_sent_reply(message): + reply = services.create_reply_draft(message=message, body="v1") + reply.status = InboxReply.Status.SENT + reply.save(update_fields=["status"]) + with pytest.raises(services.ReplyStateError): + services.discard_reply_draft(reply) + + +def test_send_reply_now_success(message, user): + reply = services.create_reply_draft(message=message, body="answer", author=user) + with patch("apps.inbox.services._dispatch_to_platform", return_value="plat-123"): + services.send_reply_now(reply) + reply.refresh_from_db() + assert reply.status == InboxReply.Status.SENT + assert reply.platform_reply_id == "plat-123" + assert reply.sent_at is not None + + +def test_send_reply_now_failure_marks_failed_and_reraises(message): + reply = services.create_reply_draft(message=message, body="answer") + with ( + patch("apps.inbox.services._dispatch_to_platform", side_effect=RuntimeError("nope")), + pytest.raises(RuntimeError), + ): + services.send_reply_now(reply) + reply.refresh_from_db() + assert reply.status == InboxReply.Status.FAILED + assert reply.send_error # human-readable reason recorded + assert reply.sent_at is None + + +def test_failed_reply_can_be_retried(message): + reply = services.create_reply_draft(message=message, body="answer") + with ( + patch("apps.inbox.services._dispatch_to_platform", side_effect=RuntimeError("nope")), + pytest.raises(RuntimeError), + ): + services.send_reply_now(reply) + with patch("apps.inbox.services._dispatch_to_platform", return_value="ok-1"): + services.send_reply_now(reply) + reply.refresh_from_db() + assert reply.status == InboxReply.Status.SENT + assert reply.send_error == "" + assert reply.platform_reply_id == "ok-1" + + +def test_provider_without_reply_api_records_locally(message): + reply = services.create_reply_draft(message=message, body="answer") + with patch("apps.inbox.services._dispatch_to_platform", side_effect=NotImplementedError): + services.send_reply_now(reply) + reply.refresh_from_db() + assert reply.status == InboxReply.Status.SENT + assert reply.platform_reply_id == "" + + +def test_send_applies_sla_auto_resolve(message): + InboxSLAConfig.objects.create(workspace=message.workspace, is_active=True, auto_resolve_on_reply=True) + reply = services.create_reply_draft(message=message, body="answer") + with patch("apps.inbox.services._dispatch_to_platform", return_value="x"): + services.send_reply_now(reply) + message.refresh_from_db() + assert message.status == InboxMessage.Status.RESOLVED + + +def test_send_without_sla_moves_unread_to_open(message): + assert message.status == InboxMessage.Status.UNREAD + reply = services.create_reply_draft(message=message, body="answer") + with patch("apps.inbox.services._dispatch_to_platform", return_value="x"): + services.send_reply_now(reply) + message.refresh_from_db() + assert message.status == InboxMessage.Status.OPEN + + +def test_send_reply_convenience_removes_failed_row(message): + with ( + patch("apps.inbox.services._dispatch_to_platform", side_effect=RuntimeError("boom")), + pytest.raises(RuntimeError), + ): + services.send_reply(message=message, body="answer") + assert InboxReply.objects.filter(inbox_message=message).count() == 0 diff --git a/apps/inbox/urls.py b/apps/inbox/urls.py index eec99430..3f7c6cc2 100644 --- a/apps/inbox/urls.py +++ b/apps/inbox/urls.py @@ -13,6 +13,10 @@ path("/", views.message_detail, name="message_detail"), # Reply to message path("/reply/", views.send_reply, name="send_reply"), + # Draft replies + path("/reply/draft/", views.save_reply_draft, name="save_reply_draft"), + path("replies//send/", views.send_reply_draft, name="send_reply_draft"), + path("replies//discard/", views.discard_reply_draft, name="discard_reply_draft"), # Internal notes path("/note/", views.add_note, name="add_note"), # Assignment diff --git a/apps/inbox/views.py b/apps/inbox/views.py index 4b822a4d..4f1ad64f 100644 --- a/apps/inbox/views.py +++ b/apps/inbox/views.py @@ -1,14 +1,12 @@ """Views for the Unified Social Inbox (F-3.1).""" import logging -from datetime import timedelta from django.contrib.auth.decorators import login_required from django.core.exceptions import PermissionDenied from django.db.models import Q from django.http import HttpResponse from django.shortcuts import get_object_or_404, redirect, render -from django.utils import timezone from django.views.decorators.http import require_POST from apps.members.decorators import require_permission @@ -17,8 +15,8 @@ from apps.notifications.models import EventType from apps.social_accounts.models import SocialAccount from apps.workspaces.models import Workspace -from providers import get_provider +from . import services as inbox_services from .forms import ( AssignForm, BulkActionForm, @@ -51,8 +49,12 @@ def _detail_context(workspace, message): ).select_related("user") replies = list(message.replies.select_related("author")) notes = list(message.internal_notes.select_related("author")) + # Sent replies sit in the chronological thread; drafts (and failed + # sends awaiting a retry) are pending work, surfaced by the composer. + sent_replies = [r for r in replies if r.status == InboxReply.Status.SENT] + draft_replies = [r for r in replies if r.status != InboxReply.Status.SENT] thread = sorted( - [("reply", r, r.sent_at) for r in replies] + [("note", n, n.created_at) for n in notes], + [("reply", r, r.sent_at or r.created_at) for r in sent_replies] + [("note", n, n.created_at) for n in notes], key=lambda x: x[2], ) child_messages = InboxMessage.objects.filter(parent_message=message).select_related("social_account") @@ -60,6 +62,7 @@ def _detail_context(workspace, message): "workspace": workspace, "message": message, "thread": thread, + "draft_replies": draft_replies, "child_messages": child_messages, "sla_config": sla_config, "saved_replies": saved_replies, @@ -208,68 +211,13 @@ def message_detail(request, workspace_id, message_id): # --- Reply --- -# Message types answered on a comment edge rather than a messaging endpoint. -_COMMENT_LIKE_TYPES = { - InboxMessage.MessageType.COMMENT, - InboxMessage.MessageType.MENTION, - InboxMessage.MessageType.REVIEW, -} -# Past this age Meta only accepts a reply tagged as written by a person. -HUMAN_AGENT_AFTER = timedelta(hours=24) - - -def _reply_failure_reason(exc: Exception) -> str: - """A short, actionable reason for the user. - - The platform's own error text carries internal diagnostics (trace IDs, raw - API JSON) that mean nothing to a workspace member, so it stays in the log - and the UI gets a stable sentence instead. - """ - from providers.exceptions import OAuthError, RateLimitError, TokenExpiredError - - if isinstance(exc, RateLimitError): - return "the account has hit its rate limit. Wait a few minutes and try again." - if isinstance(exc, TokenExpiredError | OAuthError): - return "the connection has expired. Reconnect the account in Workspace Settings." - return "the platform rejected it. Try again, or reconnect the account if this keeps happening." - - -def _send_platform_reply(message, body: str) -> str: - """Post ``body`` back to the platform and return the platform's reply ID. - - Raises if the platform refuses it, so the caller can avoid recording a - reply that was never delivered. - """ - from apps.publisher.engine import _resolve_publish_credentials - - account = message.social_account - provider = get_provider(account.platform, _resolve_publish_credentials(account)) - - # The messaging endpoints address a person, not a message, so carry the - # sender's platform-scoped ID alongside the original payload. - extra = dict(message.extra or {}) - if message.sender_handle: - extra.setdefault("recipient_id", message.sender_handle) - - if message.message_type in _COMMENT_LIKE_TYPES: - result = provider.reply_to_comment( - access_token=account.oauth_access_token, - comment_id=message.platform_message_id, - text=body, - extra=extra, - ) - else: - overdue = timezone.now() - message.received_at > HUMAN_AGENT_AFTER - result = provider.reply_to_message( - access_token=account.oauth_access_token, - message_id=message.platform_message_id, - text=body, - extra=extra, - human_agent=overdue, - ) - - return result.platform_message_id +def _get_workspace_reply(workspace, reply_id): + return get_object_or_404( + InboxReply.objects.select_related("inbox_message", "inbox_message__social_account", "author"), + id=reply_id, + inbox_message__workspace=workspace, + ) @login_required @@ -290,13 +238,7 @@ def send_reply(request, workspace_id, message_id): # A reply is only recorded if the platform accepted it. Recording it # regardless would show the team a sent reply the customer never got. try: - platform_reply_id = _send_platform_reply(message, body) - except NotImplementedError: - # The platform has no reply API (or none for this item type). Keep the - # reply as a local record so the team still has their answer on file — - # this is what the inbox did before replies were sent for real. - logger.info("Provider %s cannot send replies; recording locally.", account.platform) - platform_reply_id = "" + reply = inbox_services.send_reply(message=message, body=body, author=request.user) except Exception as exc: logger.exception("Failed to send reply for message %s (%s)", message.id, account.platform) response = render( @@ -304,7 +246,7 @@ def send_reply(request, workspace_id, message_id): "inbox/partials/_reply_error.html", { "platform_label": account.get_platform_display(), - "reason": _reply_failure_reason(exc), + "reason": inbox_services._reply_failure_reason(exc), }, ) # htmx does not swap on a 4xx/5xx, so the failure is reported as a @@ -312,26 +254,80 @@ def send_reply(request, workspace_id, message_id): response["HX-Reply-Failed"] = "1" return response - reply = InboxReply.objects.create( - inbox_message=message, - author=request.user, - body=body, - platform_reply_id=platform_reply_id, - ) - - # Auto-resolve on reply if configured - sla_config = InboxSLAConfig.objects.filter(workspace=workspace, is_active=True).first() - if sla_config and sla_config.auto_resolve_on_reply: - message.status = InboxMessage.Status.RESOLVED - message.save(update_fields=["status"]) - elif message.status == InboxMessage.Status.UNREAD: - message.status = InboxMessage.Status.OPEN - message.save(update_fields=["status"]) - context = {"reply": reply, "workspace": workspace, "message": message} return render(request, "inbox/partials/_reply_item.html", context) +@login_required +@require_permission("use_inbox") +@require_POST +def save_reply_draft(request, workspace_id, message_id): + """Save a reply as a draft without sending it.""" + workspace = _get_workspace(request, workspace_id) + message = get_object_or_404(InboxMessage, id=message_id, workspace=workspace) + + form = ReplyForm(request.POST) + if not form.is_valid(): + return HttpResponse("Invalid reply.", status=400) + + try: + inbox_services.create_reply_draft( + message=message, + body=form.cleaned_data["body"], + author=request.user, + ) + except ValueError as exc: + return HttpResponse(str(exc), status=400) + + message.refresh_from_db() + return render(request, "inbox/partials/_message_panel.html", _detail_context(workspace, message)) + + +@login_required +@require_permission("reply_from_inbox") +@require_POST +def send_reply_draft(request, workspace_id, reply_id): + """Deliver an existing draft reply to the platform.""" + workspace = _get_workspace(request, workspace_id) + reply = _get_workspace_reply(workspace, reply_id) + message = reply.inbox_message + + failed = False + try: + inbox_services.send_reply_now(reply, actor=request.user) + except inbox_services.ReplyStateError as exc: + return HttpResponse(str(exc), status=409) + except Exception: + # The draft is kept (now in ``failed`` state) so the team can retry + # or discard it; the re-rendered panel shows it with failed styling. + logger.exception("Failed to send draft reply %s", reply.id) + failed = True + + message.refresh_from_db() + panel = render(request, "inbox/partials/_message_panel.html", _detail_context(workspace, message)) + if failed: + panel["HX-Reply-Failed"] = "1" + return panel + + +@login_required +@require_permission("use_inbox") +@require_POST +def discard_reply_draft(request, workspace_id, reply_id): + """Delete a draft (or failed) reply.""" + workspace = _get_workspace(request, workspace_id) + reply = _get_workspace_reply(workspace, reply_id) + message = reply.inbox_message + + try: + inbox_services.discard_reply_draft(reply) + except inbox_services.ReplyStateError as exc: + return HttpResponse(str(exc), status=409) + + message.refresh_from_db() + return render(request, "inbox/partials/_message_panel.html", _detail_context(workspace, message)) + + # --- Internal Note --- diff --git a/apps/mcp/handlers.py b/apps/mcp/handlers.py index 0ed2f52d..35934294 100644 --- a/apps/mcp/handlers.py +++ b/apps/mcp/handlers.py @@ -28,6 +28,14 @@ from apps.api.schemas import PostResponse from apps.composer.models import PlatformPost, Post from apps.composer.services import create_post, transition_platform_post +from apps.inbox.models import InboxMessage, InboxReply +from apps.inbox.services import ( + ReplyStateError, + create_reply_draft, + discard_reply_draft, + send_reply_now, + update_reply_draft, +) from apps.mcp.protocol import INVALID_PARAMS, JsonRpcError from apps.mcp.tools import Tool, register_tool from apps.social_accounts.models import SocialAccount @@ -1208,3 +1216,375 @@ def _get_post_analytics(args: dict, context: dict[str, Any]) -> dict: handler=_get_post_analytics, ) ) + + +# --------------------------------------------------------------------------- +# Inbox: shared helpers +# --------------------------------------------------------------------------- + + +def _inbox_allowed_account_ids(api_key) -> list: + return [sa.id for sa in api_key.social_accounts.all()] + + +def _visible_inbox_qs(api_key): + """InboxMessages this key may see: in its workspace, on an allowlisted account. + + Fails closed on an empty allowlist rather than relying on ``__in=[]`` folding. + """ + allowed = _inbox_allowed_account_ids(api_key) + if not allowed: + return InboxMessage.objects.none() + return InboxMessage.objects.filter( + workspace_id=api_key.workspace_id, + social_account_id__in=allowed, + ).select_related("social_account") + + +def _get_inbox_message_for_key(api_key, message_id_str: str) -> InboxMessage: + message_id = _parse_uuid(message_id_str, "message_id") + try: + return _visible_inbox_qs(api_key).prefetch_related("replies__author").get(id=message_id) + except InboxMessage.DoesNotExist as exc: + raise JsonRpcError(INVALID_PARAMS, "Inbox message not found") from exc + + +def _get_inbox_reply_for_key(api_key, reply_id_str: str) -> InboxReply: + reply_id = _parse_uuid(reply_id_str, "reply_id") + allowed = _inbox_allowed_account_ids(api_key) + try: + reply = InboxReply.objects.select_related("inbox_message", "inbox_message__social_account", "author").get( + id=reply_id, inbox_message__workspace_id=api_key.workspace_id + ) + except InboxReply.DoesNotExist as exc: + raise JsonRpcError(INVALID_PARAMS, "Reply not found") from exc + if reply.inbox_message.social_account_id not in allowed: + raise JsonRpcError(INVALID_PARAMS, "Reply not found") + return reply + + +def _serialize_inbox_message(message: InboxMessage) -> dict: + from apps.api.schemas import InboxMessageResponse + + return InboxMessageResponse.from_message(message, include_replies=True).model_dump(mode="json") + + +def _serialize_inbox_reply(reply: InboxReply) -> dict: + from apps.api.schemas import InboxReplyResponse + + return InboxReplyResponse.from_reply(reply).model_dump(mode="json") + + +# --------------------------------------------------------------------------- +# Tool: list_inbox_messages +# --------------------------------------------------------------------------- + + +_MCP_INBOX_LIMIT_DEFAULT = 50 +_MCP_INBOX_LIMIT_MAX = 100 + + +def _list_inbox_messages(args: dict, context: dict[str, Any]) -> dict: + _require_perm(context, "use_inbox") + api_key = context["api_key"] + + status = args.get("status") + if status is not None and status not in InboxMessage.Status.values: + raise JsonRpcError(INVALID_PARAMS, f"status must be one of {', '.join(InboxMessage.Status.values)}") + message_type = args.get("message_type") + if message_type is not None and message_type not in InboxMessage.MessageType.values: + raise JsonRpcError(INVALID_PARAMS, f"message_type must be one of {', '.join(InboxMessage.MessageType.values)}") + + raw_limit = args.get("limit") + try: + limit = _MCP_INBOX_LIMIT_DEFAULT if raw_limit is None else int(raw_limit) + except (TypeError, ValueError) as exc: + raise JsonRpcError(INVALID_PARAMS, f"limit must be an integer between 1 and {_MCP_INBOX_LIMIT_MAX}") from exc + if limit < 1 or limit > _MCP_INBOX_LIMIT_MAX: + raise JsonRpcError(INVALID_PARAMS, f"limit must be between 1 and {_MCP_INBOX_LIMIT_MAX}") + + try: + offset = decode_offset_cursor(args.get("cursor")) + except ValueError as exc: + raise JsonRpcError(INVALID_PARAMS, "cursor is not a valid pagination cursor") from exc + + qs = _visible_inbox_qs(api_key).prefetch_related("replies__author") + if status: + qs = qs.filter(status=status) + if message_type: + qs = qs.filter(message_type=message_type) + sa_id = args.get("social_account_id") + if sa_id is not None: + sa_uuid = _parse_uuid(sa_id, "social_account_id") + if sa_uuid not in set(_inbox_allowed_account_ids(api_key)): + raise JsonRpcError(INVALID_PARAMS, "social_account_id is not in this API key's allowlist") + qs = qs.filter(social_account_id=sa_uuid) + qs = qs.order_by("-received_at", "id") + + rows = list(qs[offset : offset + limit + 1]) + has_more = len(rows) > limit + rows = rows[:limit] + return _wrap_text( + { + "messages": [_serialize_inbox_message(m) for m in rows], + "limit": limit, + "next_cursor": encode_offset_cursor(offset + limit) if has_more else None, + } + ) + + +register_tool( + Tool( + name="list_inbox_messages", + description=( + "List inbound inbox items (comments, mentions, DMs, reviews) for the accounts this " + "API key is allowed to act on, newest first. Each item carries its reply thread " + "(including any draft replies). Optional `status` (unread/open/resolved/archived), " + "`message_type` (comment/mention/dm/review) and `social_account_id` filters, plus " + "`limit` (default 50, max 100). When more remain, `next_cursor` is non-null — pass it " + "back as `cursor`. Requires the use_inbox permission." + ), + input_schema={ + "type": "object", + "properties": { + "status": {"type": "string", "enum": list(InboxMessage.Status.values)}, + "message_type": {"type": "string", "enum": list(InboxMessage.MessageType.values)}, + "social_account_id": {"type": "string", "format": "uuid"}, + "limit": { + "type": "integer", + "minimum": 1, + "maximum": _MCP_INBOX_LIMIT_MAX, + "default": _MCP_INBOX_LIMIT_DEFAULT, + }, + "cursor": {"type": "string", "description": "Opaque cursor from a previous call's next_cursor."}, + }, + "additionalProperties": False, + }, + handler=_list_inbox_messages, + ) +) + + +# --------------------------------------------------------------------------- +# Tool: get_inbox_message +# --------------------------------------------------------------------------- + + +def _get_inbox_message(args: dict, context: dict[str, Any]) -> dict: + _require_perm(context, "use_inbox") + if "message_id" not in args: + raise JsonRpcError(INVALID_PARAMS, "message_id is required") + message = _get_inbox_message_for_key(context["api_key"], args["message_id"]) + return _wrap_text(_serialize_inbox_message(message)) + + +register_tool( + Tool( + name="get_inbox_message", + description=( + "Retrieve one inbox message by ID, including its full reply thread and any draft " + "replies. Returns 'Inbox message not found' for IDs outside this key's workspace or " + "account allowlist (same as a truly nonexistent ID). Requires the use_inbox permission." + ), + input_schema={ + "type": "object", + "properties": {"message_id": {"type": "string", "format": "uuid"}}, + "required": ["message_id"], + "additionalProperties": False, + }, + handler=_get_inbox_message, + ) +) + + +# --------------------------------------------------------------------------- +# Tool: create_reply_draft +# --------------------------------------------------------------------------- + + +def _create_reply_draft(args: dict, context: dict[str, Any]) -> dict: + _require_perm(context, "use_inbox") + api_key = context["api_key"] + if "message_id" not in args: + raise JsonRpcError(INVALID_PARAMS, "message_id is required") + if not args.get("body"): + raise JsonRpcError(INVALID_PARAMS, "body is required") + message = _get_inbox_message_for_key(api_key, args["message_id"]) + try: + reply = create_reply_draft( + message=message, + body=args["body"], + author=api_key.issued_by if api_key.issued_by_id else None, + ) + except ValueError as exc: + raise JsonRpcError(INVALID_PARAMS, str(exc)) from exc + return _wrap_text(_serialize_inbox_reply(reply)) + + +register_tool( + Tool( + name="create_reply_draft", + description=( + "Draft a reply to an inbox message. The draft is saved but NOT sent to the platform; " + "a human can review it in the inbox, or call send_reply to deliver it. Requires the " + "use_inbox permission (drafting is not sending)." + ), + input_schema={ + "type": "object", + "properties": { + "message_id": {"type": "string", "format": "uuid"}, + "body": {"type": "string", "minLength": 1, "maxLength": 10000}, + }, + "required": ["message_id", "body"], + "additionalProperties": False, + }, + handler=_create_reply_draft, + ) +) + + +# --------------------------------------------------------------------------- +# Tool: update_reply_draft +# --------------------------------------------------------------------------- + + +def _update_reply_draft(args: dict, context: dict[str, Any]) -> dict: + _require_perm(context, "use_inbox") + if "reply_id" not in args: + raise JsonRpcError(INVALID_PARAMS, "reply_id is required") + if not args.get("body"): + raise JsonRpcError(INVALID_PARAMS, "body is required") + reply = _get_inbox_reply_for_key(context["api_key"], args["reply_id"]) + try: + update_reply_draft(reply, body=args["body"]) + except (ReplyStateError, ValueError) as exc: + raise JsonRpcError(INVALID_PARAMS, str(exc)) from exc + return _wrap_text(_serialize_inbox_reply(reply)) + + +register_tool( + Tool( + name="update_reply_draft", + description=( + "Replace the body of an existing draft (or failed) reply. Sent replies cannot be " + "edited. Requires the use_inbox permission." + ), + input_schema={ + "type": "object", + "properties": { + "reply_id": {"type": "string", "format": "uuid"}, + "body": {"type": "string", "minLength": 1, "maxLength": 10000}, + }, + "required": ["reply_id", "body"], + "additionalProperties": False, + }, + handler=_update_reply_draft, + ) +) + + +# --------------------------------------------------------------------------- +# Tool: discard_reply_draft +# --------------------------------------------------------------------------- + + +def _discard_reply_draft(args: dict, context: dict[str, Any]) -> dict: + _require_perm(context, "use_inbox") + if "reply_id" not in args: + raise JsonRpcError(INVALID_PARAMS, "reply_id is required") + reply = _get_inbox_reply_for_key(context["api_key"], args["reply_id"]) + reply_id = str(reply.id) + try: + discard_reply_draft(reply) + except ReplyStateError as exc: + raise JsonRpcError(INVALID_PARAMS, str(exc)) from exc + return _wrap_text({"discarded": True, "reply_id": reply_id}) + + +register_tool( + Tool( + name="discard_reply_draft", + description=( + "Delete a draft (or failed) reply. Sent replies are permanent and cannot be " + "discarded. Requires the use_inbox permission." + ), + input_schema={ + "type": "object", + "properties": {"reply_id": {"type": "string", "format": "uuid"}}, + "required": ["reply_id"], + "additionalProperties": False, + }, + handler=_discard_reply_draft, + ) +) + + +# --------------------------------------------------------------------------- +# Tool: send_reply +# --------------------------------------------------------------------------- + + +def _send_reply(args: dict, context: dict[str, Any]) -> dict: + # Sending pushes text to the real platform on the workspace's behalf, + # so it needs the stronger inbox permission — same split as the web UI. + _require_perm(context, "reply_from_inbox") + api_key = context["api_key"] + actor = api_key.issued_by if api_key.issued_by_id else None + + reply_id = args.get("reply_id") + if reply_id: + reply = _get_inbox_reply_for_key(api_key, reply_id) + else: + if "message_id" not in args or not args.get("body"): + raise JsonRpcError( + INVALID_PARAMS, + "Provide either reply_id (to send an existing draft) or message_id + body", + ) + message = _get_inbox_message_for_key(api_key, args["message_id"]) + try: + reply = create_reply_draft(message=message, body=args["body"], author=actor) + except ValueError as exc: + raise JsonRpcError(INVALID_PARAMS, str(exc)) from exc + + try: + send_reply_now(reply, actor=actor) + except NotImplementedError: + # Provider has no reply API; the reply is recorded locally as sent. + pass + except ReplyStateError as exc: + raise JsonRpcError(INVALID_PARAMS, str(exc)) from exc + except Exception as exc: # platform refused it — reply is left in "failed" + raise JsonRpcError(INVALID_PARAMS, f"Reply not sent: {exc}") from exc + + return _wrap_text(_serialize_inbox_reply(reply)) + + +register_tool( + Tool( + name="send_reply", + description=( + "Deliver a reply to an inbox message's platform. Either pass `reply_id` to send an " + "existing draft, or `message_id` + `body` to create and send in one step. On a " + "platform refusal the reply is kept in `failed` state (retry with the same reply_id) " + "and an error is returned. Requires the reply_from_inbox permission." + ), + input_schema={ + "type": "object", + "properties": { + "reply_id": { + "type": "string", + "format": "uuid", + "description": "ID of an existing draft/failed reply to send.", + }, + "message_id": { + "type": "string", + "format": "uuid", + "description": "Inbox message to reply to (with `body`) when not using `reply_id`.", + }, + "body": {"type": "string", "minLength": 1, "maxLength": 10000}, + }, + "additionalProperties": False, + }, + handler=_send_reply, + ) +) diff --git a/apps/mcp/tests/test_inbox_tools.py b/apps/mcp/tests/test_inbox_tools.py new file mode 100644 index 00000000..4c47eb95 --- /dev/null +++ b/apps/mcp/tests/test_inbox_tools.py @@ -0,0 +1,264 @@ +"""MCP inbox tools: list / get messages, draft / send / discard replies.""" + +from __future__ import annotations + +import json +from datetime import timedelta +from unittest.mock import patch + +import pytest +from django.test import Client +from django.utils import timezone + +from apps.api_keys import services +from apps.inbox.models import InboxMessage, InboxReply +from apps.mcp.protocol import INVALID_PARAMS +from apps.members.models import PERMISSION_KEYS, OrgMembership, WorkspaceMembership + +MCP_URL = "/api/v1/mcp/" + + +class _SecureClient(Client): + def generic(self, method, path, *args, **kwargs): + kwargs["secure"] = True + return super().generic(method, path, *args, **kwargs) + + +def _rpc(name: str, arguments: dict) -> dict: + return { + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": {"name": name, "arguments": arguments}, + } + + +def _call(client: Client, name: str, arguments: dict): + r = client.post(MCP_URL, data=json.dumps(_rpc(name, arguments)), content_type="application/json") + return r.status_code, r.json() + + +def _result_json(body: dict) -> dict: + return json.loads(body["result"]["content"][0]["text"]) + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture +def user(db): + from apps.accounts.models import User + + return User.objects.create_user( + email="mcp-inbox@example.com", password="x", name="MCP Inbox", tos_accepted_at=timezone.now() + ) + + +@pytest.fixture +def organization(db): + from apps.organizations.models import Organization + + return Organization.objects.create(name="Org") + + +@pytest.fixture +def workspace(db, organization): + from apps.workspaces.models import Workspace + + return Workspace.objects.create(name="WS", organization=organization) + + +@pytest.fixture +def memberships(db, user, organization, workspace): + OrgMembership.objects.create(user=user, organization=organization, org_role=OrgMembership.OrgRole.OWNER) + return WorkspaceMembership.objects.create( + user=user, workspace=workspace, workspace_role=WorkspaceMembership.WorkspaceRole.OWNER + ) + + +@pytest.fixture +def account(db, workspace): + from apps.social_accounts.models import SocialAccount + + return SocialAccount.objects.create( + workspace=workspace, + platform="facebook", + account_platform_id="page-1", + account_name="Page", + connection_status="connected", + oauth_access_token="tok", + ) + + +@pytest.fixture +def other_account(db, workspace): + from apps.social_accounts.models import SocialAccount + + return SocialAccount.objects.create( + workspace=workspace, + platform="facebook", + account_platform_id="page-2", + account_name="Page 2", + connection_status="connected", + oauth_access_token="tok2", + ) + + +@pytest.fixture +def message(db, account): + return InboxMessage.objects.create( + workspace=account.workspace, + social_account=account, + platform_message_id="pm-1", + message_type=InboxMessage.MessageType.COMMENT, + sender_name="Ada", + sender_handle="ada", + body="hi?", + received_at=timezone.now() - timedelta(hours=1), + ) + + +@pytest.fixture +def full_client(db, user, memberships, workspace, account): + key = services.issue_api_key( + workspace=workspace, + social_accounts=[account], + issued_by=user, + name="full", + permissions=list(PERMISSION_KEYS), + ) + return _SecureClient(HTTP_AUTHORIZATION=f"Bearer {key.plaintext_token}") + + +@pytest.fixture +def draft_only_client(db, user, memberships, workspace, account): + key = services.issue_api_key( + workspace=workspace, + social_accounts=[account], + issued_by=user, + name="draft-only", + permissions=["use_inbox"], + ) + return _SecureClient(HTTP_AUTHORIZATION=f"Bearer {key.plaintext_token}") + + +# --------------------------------------------------------------------------- +# Tests +# --------------------------------------------------------------------------- + + +@pytest.mark.django_db +class TestInboxReadTools: + def test_list_inbox_messages(self, full_client, message): + InboxReply.objects.create(inbox_message=message, body="a draft") + _s, body = _call(full_client, "list_inbox_messages", {}) + data = _result_json(body) + assert len(data["messages"]) == 1 + assert data["messages"][0]["replies"][0]["body"] == "a draft" + + def test_list_excludes_non_allowlisted_account(self, full_client, message, other_account): + InboxMessage.objects.create( + workspace=other_account.workspace, + social_account=other_account, + platform_message_id="pm-x", + message_type=InboxMessage.MessageType.COMMENT, + sender_name="X", + body="?", + received_at=timezone.now(), + ) + _s, body = _call(full_client, "list_inbox_messages", {}) + data = _result_json(body) + assert {m["id"] for m in data["messages"]} == {str(message.id)} + + def test_get_inbox_message(self, full_client, message): + _s, body = _call(full_client, "get_inbox_message", {"message_id": str(message.id)}) + assert _result_json(body)["id"] == str(message.id) + + def test_get_unknown_message_errors(self, full_client): + import uuid + + _s, body = _call(full_client, "get_inbox_message", {"message_id": str(uuid.uuid4())}) + assert body["error"]["code"] == INVALID_PARAMS + assert "not found" in body["error"]["message"].lower() + + +@pytest.mark.django_db +class TestInboxReplyTools: + def test_create_reply_draft(self, full_client, message): + _s, body = _call(full_client, "create_reply_draft", {"message_id": str(message.id), "body": "hi"}) + data = _result_json(body) + assert data["status"] == "draft" + assert InboxReply.objects.get(id=data["id"]).body == "hi" + + def test_create_reply_draft_needs_use_inbox(self, db, user, memberships, workspace, account, message): + key = services.issue_api_key( + workspace=workspace, + social_accounts=[account], + issued_by=user, + name="none", + permissions=["view_analytics"], + ) + c = _SecureClient(HTTP_AUTHORIZATION=f"Bearer {key.plaintext_token}") + _s, body = _call(c, "create_reply_draft", {"message_id": str(message.id), "body": "hi"}) + assert body["error"]["code"] == INVALID_PARAMS + assert "permission denied" in body["error"]["message"].lower() + + def test_update_reply_draft(self, full_client, message): + reply = InboxReply.objects.create(inbox_message=message, body="v1") + _s, body = _call(full_client, "update_reply_draft", {"reply_id": str(reply.id), "body": "v2"}) + assert _result_json(body)["body"] == "v2" + + def test_discard_reply_draft(self, full_client, message): + reply = InboxReply.objects.create(inbox_message=message, body="scrap") + _s, body = _call(full_client, "discard_reply_draft", {"reply_id": str(reply.id)}) + assert _result_json(body)["discarded"] is True + assert not InboxReply.objects.filter(pk=reply.pk).exists() + + def test_send_reply_with_reply_id(self, full_client, message): + reply = InboxReply.objects.create(inbox_message=message, body="ready") + with patch("apps.inbox.services._dispatch_to_platform", return_value="plat-1"): + _s, body = _call(full_client, "send_reply", {"reply_id": str(reply.id)}) + assert _result_json(body)["status"] == "sent" + + def test_send_reply_create_and_send(self, full_client, message): + with patch("apps.inbox.services._dispatch_to_platform", return_value="plat-2"): + _s, body = _call(full_client, "send_reply", {"message_id": str(message.id), "body": "yo"}) + data = _result_json(body) + assert data["status"] == "sent" + assert data["platform_reply_id"] == "plat-2" + + def test_send_reply_requires_reply_from_inbox(self, draft_only_client, message): + reply = InboxReply.objects.create(inbox_message=message, body="ready") + _s, body = _call(draft_only_client, "send_reply", {"reply_id": str(reply.id)}) + assert body["error"]["code"] == INVALID_PARAMS + assert "permission denied: reply_from_inbox" in body["error"]["message"].lower() + + def test_draft_only_client_can_create_draft(self, draft_only_client, message): + _s, body = _call(draft_only_client, "create_reply_draft", {"message_id": str(message.id), "body": "d"}) + assert _result_json(body)["status"] == "draft" + + def test_send_reply_platform_failure_is_reshaped(self, full_client, message): + reply = InboxReply.objects.create(inbox_message=message, body="ready") + with patch("apps.inbox.services._dispatch_to_platform", side_effect=RuntimeError("no")): + _s, body = _call(full_client, "send_reply", {"reply_id": str(reply.id)}) + assert body["error"]["code"] == INVALID_PARAMS + assert "reply not sent" in body["error"]["message"].lower() + reply.refresh_from_db() + assert reply.status == InboxReply.Status.FAILED + + def test_reply_on_foreign_account_is_not_found(self, full_client, other_account): + m = InboxMessage.objects.create( + workspace=other_account.workspace, + social_account=other_account, + platform_message_id="pm-x", + message_type=InboxMessage.MessageType.COMMENT, + sender_name="X", + body="?", + received_at=timezone.now(), + ) + reply = InboxReply.objects.create(inbox_message=m, body="x") + _s, body = _call(full_client, "update_reply_draft", {"reply_id": str(reply.id), "body": "y"}) + assert body["error"]["code"] == INVALID_PARAMS + assert "not found" in body["error"]["message"].lower() diff --git a/apps/mcp/tests/test_rest_parity.py b/apps/mcp/tests/test_rest_parity.py index d4970b57..23dcb6fd 100644 --- a/apps/mcp/tests/test_rest_parity.py +++ b/apps/mcp/tests/test_rest_parity.py @@ -488,3 +488,76 @@ def test_create_draft_naive_proposed_stored_as_utc(self, client_with_token, soci ) # A tz-less value is interpreted as UTC and serialized with a Z suffix. assert created["proposed_publish_at"] == "2027-09-01T09:00:00Z" + + +# --------------------------------------------------------------------------- +# Inbox parity — REST /inbox and the MCP inbox tools share +# ``InboxMessageResponse`` / ``InboxReplyResponse``, so payloads must match. +# --------------------------------------------------------------------------- + + +@pytest.fixture +def inbox_message(db, workspace, social_account): + from apps.inbox.models import InboxMessage + + return InboxMessage.objects.create( + workspace=workspace, + social_account=social_account, + platform_message_id="pm-parity-1", + message_type="comment", + sender_name="Commenter", + sender_handle="commenter", + body="Nice post!", + received_at=timezone.now() - timedelta(hours=1), + ) + + +@pytest.mark.django_db +class TestRestMcpInboxParity: + def test_get_inbox_message_bodies_match(self, client_with_token, inbox_message): + from apps.inbox.models import InboxReply + + InboxReply.objects.create(inbox_message=inbox_message, body="a draft reply") + + rest = client_with_token.get(f"/api/v1/inbox/{inbox_message.id}") + assert rest.status_code == 200, rest.content + rest_body = rest.json() + + mcp = client_with_token.post( + MCP_URL, + data=json.dumps( + _rpc( + "tools/call", + {"name": "get_inbox_message", "arguments": {"message_id": str(inbox_message.id)}}, + ) + ), + content_type="application/json", + ) + assert mcp.status_code == 200 + envelope = mcp.json() + assert "error" not in envelope, envelope + mcp_body = json.loads(envelope["result"]["content"][0]["text"]) + + assert mcp_body == rest_body, ( + "MCP and REST disagree on the InboxMessageResponse payload. " + "Both surfaces must call InboxMessageResponse.from_message." + ) + + def test_list_inbox_messages_matches_rest_list(self, client_with_token, inbox_message): + from apps.inbox.models import InboxReply + + InboxReply.objects.create(inbox_message=inbox_message, body="draft") + + rest = client_with_token.get("/api/v1/inbox/") + assert rest.status_code == 200, rest.content + rest_body = rest.json() + + mcp = client_with_token.post( + MCP_URL, + data=json.dumps(_rpc("tools/call", {"name": "list_inbox_messages", "arguments": {}})), + content_type="application/json", + ) + assert mcp.status_code == 200 + mcp_body = json.loads(mcp.json()["result"]["content"][0]["text"]) + + assert mcp_body == rest_body diff --git a/pyproject.toml b/pyproject.toml index ba44d3a9..941af092 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,6 +28,7 @@ ignore = [ # doesn't apply. "apps/api/routers/media.py" = ["B008"] "apps/api/routers/analytics.py" = ["B008"] +"apps/api/routers/inbox.py" = ["B008"] [tool.ruff.lint.isort] known-first-party = ["apps", "config"] diff --git a/templates/inbox/partials/_draft_reply_item.html b/templates/inbox/partials/_draft_reply_item.html new file mode 100644 index 00000000..72c3dece --- /dev/null +++ b/templates/inbox/partials/_draft_reply_item.html @@ -0,0 +1,33 @@ +{% load humanize %} +{# One pending reply: a draft awaiting review, or a failed send awaiting retry. #} +
+
+ {% if draft.status == 'failed' %} + Failed + {% else %} + Draft + {% endif %} + {{ draft.author.get_short_name|default:draft.author.email|default:"—" }} + {{ draft.created_at|timesince }} ago +
+

{{ draft.body }}

+ {% if draft.status == 'failed' and draft.send_error %} +

Last attempt failed: {{ draft.send_error }}

+ {% endif %} +
+ + +
+
diff --git a/templates/inbox/partials/_reply_composer.html b/templates/inbox/partials/_reply_composer.html index 2e3cb57d..9e20d1a7 100644 --- a/templates/inbox/partials/_reply_composer.html +++ b/templates/inbox/partials/_reply_composer.html @@ -3,6 +3,16 @@
+ + {% if draft_replies %} +
+

Pending replies

+ {% for draft in draft_replies %} + {% include "inbox/partials/_draft_reply_item.html" with draft=draft %} + {% endfor %} +
+ {% endif %} +
+
+ + +