Feature/report comments

.claude/CLAUDE.md

@@ -36,7 +36,7 @@ ddpui/
 │
 ├── core/
 │   └── {module}/                    # Feature module (all business logic here)
-│       ├── __init__.py
+│       ├── __init__.py              # Keep empty (no re-exports or __all__)
 │       ├── {module}_service.py      # Business logic and orchestration
 │       ├── {module}_operations.py   # Domain operations (optional)
 │       └── exceptions.py            # Custom exceptions for this feature
@@ -297,7 +297,7 @@ def delete_{module}(request, id: int):
 
 ```
 ddpui/core/{module}/
-├── __init__.py              # Export public interfaces
+├── __init__.py              # Keep empty — do NOT add re-exports or __all__
 ├── {module}_service.py      # Business logic and orchestration
 ├── {module}_operations.py   # Domain-specific operations (optional)
 └── exceptions.py            # Custom exceptions
@@ -1105,15 +1105,9 @@ ddpui/
 
 #### `core/charts/__init__.py`
 ```python
-from .chart_service import ChartService
-from .exceptions import ChartError, ChartNotFoundError, ChartValidationError
-
-__all__ = [
-    "ChartService",
-    "ChartError",
-    "ChartNotFoundError",
-    "ChartValidationError",
-]
+# Keep empty — all imports should use full paths like:
+# from ddpui.core.charts.chart_service import ChartService
+# Do NOT add re-exports or __all__ here.
 ```
 
 #### `exceptions.py`

ddpui/api/report_api.py

@@ -1,14 +1,17 @@
 """Report API endpoints"""
 
-import secrets
 from typing import List
 
 from django.http import HttpResponse
 from ninja import Router
 from ninja.errors import HttpError
 
 from ddpui.auth import has_permission
+from ddpui.core.reports.comment_service import CommentService
 from ddpui.core.reports.exceptions import (
+    CommentNotFoundError,
+    CommentPermissionError,
+    CommentValidationError,
     SnapshotNotFoundError,
     SnapshotPermissionError,
     SnapshotValidationError,
@@ -19,7 +22,13 @@
 from ddpui.models.org_user import OrgUser
 from ddpui.schemas.dashboard_schema import ShareResponse, ShareStatus, ShareToggle
 from ddpui.schemas.report_schema import (
+    CommentCreate,
+    CommentResponse,
+    CommentStatesResponse,
+    CommentUpdate,
     DatetimeColumnResponse,
+    MarkReadRequest,
+    MentionableUserResponse,
     SnapshotCreate,
     SnapshotDeleteResponse,
     SnapshotResponse,
@@ -80,6 +89,19 @@ def create_snapshot(request, payload: SnapshotCreate):
         raise HttpError(500, "Failed to create snapshot") from e
 
 
+@report_router.get("/mentionable-users/", response=ApiResponse[List[MentionableUserResponse]])
+@has_permission(["can_view_dashboards"])
+def get_mentionable_users(request):
+    """List org users available for @mention"""
+    orguser: OrgUser = request.orguser
+
+    users = CommentService.get_mentionable_users(orguser.org)
+    return api_response(
+        success=True,
+        data=[MentionableUserResponse.from_orguser(u) for u in users],
+    )
+
+
 @report_router.get("/{snapshot_id}/view/", response=ApiResponse[SnapshotViewResponse])
 @has_permission(["can_view_dashboards"])
 def get_snapshot_view(request, snapshot_id: int):
@@ -144,14 +166,8 @@ def export_report_pdf(request, snapshot_id: int):
         raise HttpError(404, str(err)) from err
 
     try:
-        # Ensure the snapshot has a share token (needed for the URL).
-        # This does NOT make the report publicly accessible — Playwright
-        # authenticates via the render secret header, not is_public.
-        if not snapshot.public_share_token:
-            snapshot.public_share_token = secrets.token_urlsafe(48)
-            snapshot.save(update_fields=["public_share_token"])
-
-        pdf_bytes = PdfExportService.generate_pdf(snapshot_id, snapshot.public_share_token)
+        share_token = ReportService.ensure_share_token(snapshot)
+        pdf_bytes = PdfExportService.generate_pdf(snapshot_id, share_token)
 
         safe_title = "".join(c for c in snapshot.title if c.isalnum() or c in " -_").strip()
         filename = f"{safe_title or 'report'}.pdf"
@@ -217,3 +233,143 @@ def get_report_sharing_status(request, snapshot_id: int):
         raise HttpError(404, str(err)) from err
     except SnapshotPermissionError as err:
         raise HttpError(403, str(err)) from err
+
+
+# ===== Comment Endpoints (nested under /{snapshot_id}/comments/) =====
+
+
+@report_router.get("/{snapshot_id}/comments/states/", response=ApiResponse[CommentStatesResponse])
+@has_permission(["can_view_dashboards"])
+def get_comment_states(request, snapshot_id: int):
+    """Get icon states for all targets in a snapshot"""
+    orguser: OrgUser = request.orguser
+
+    try:
+        states = CommentService.get_comment_states(
+            snapshot_id=snapshot_id,
+            org=orguser.org,
+            orguser=orguser,
+        )
+        return api_response(
+            success=True,
+            data=CommentStatesResponse(states=states),
+        )
+    except CommentValidationError as err:
+        raise HttpError(400, str(err)) from err
+
+
+@report_router.post("/{snapshot_id}/comments/mark-read/", response=ApiResponse)
+@has_permission(["can_view_dashboards"])
+def mark_as_read(request, snapshot_id: int, payload: MarkReadRequest):
+    """Mark a target's comments as read"""
+    orguser: OrgUser = request.orguser
+
+    try:
+        CommentService.mark_as_read(
+            snapshot_id=snapshot_id,
+            orguser=orguser,
+            target_type=payload.target_type,
+            chart_id=payload.chart_id,
+        )
+        return api_response(success=True, message="Marked as read")
+    except CommentValidationError as err:
+        raise HttpError(400, str(err)) from err
+
+
+@report_router.get("/{snapshot_id}/comments/", response=ApiResponse[List[CommentResponse]])
+@has_permission(["can_view_dashboards"])
+def list_comments(
+    request,
+    snapshot_id: int,
+    target_type: str,
+    chart_id: int = None,
+):
+    """List comments for a report target"""
+    orguser: OrgUser = request.orguser
+
+    try:
+        comments = CommentService.list_comments(
+            snapshot_id=snapshot_id,
+            org=orguser.org,
+            target_type=target_type,
+            chart_id=chart_id,
+            orguser=orguser,
+        )
+        return api_response(
+            success=True,
+            data=[CommentResponse.from_model(c) for c in comments],
+        )
+    except CommentValidationError as err:
+        raise HttpError(400, str(err)) from err
+
+
+@report_router.post("/{snapshot_id}/comments/", response=ApiResponse[CommentResponse])
+@has_permission(["can_view_dashboards"])
+def create_comment(request, snapshot_id: int, payload: CommentCreate):
+    """Create a comment on a report snapshot"""
+    orguser: OrgUser = request.orguser
+
+    try:
+        comment = CommentService.create_comment(
+            snapshot_id=snapshot_id,
+            org=orguser.org,
+            orguser=orguser,
+            target_type=payload.target_type,
+            content=payload.content,
+            chart_id=payload.chart_id,
+            mentioned_emails=payload.mentioned_emails,
+        )
+        return api_response(
+            success=True,
+            data=CommentResponse.from_model(comment),
+            message="Comment created",
+        )
+    except CommentValidationError as err:
+        raise HttpError(400, str(err)) from err
+    except Exception as e:
+        logger.error(f"Error creating comment: {e}", exc_info=True)
+        raise HttpError(500, "Failed to create comment") from e
+
+
+@report_router.put("/{snapshot_id}/comments/{comment_id}/", response=ApiResponse[CommentResponse])
+@has_permission(["can_view_dashboards"])
+def update_comment(request, snapshot_id: int, comment_id: int, payload: CommentUpdate):
+    """Update a comment (author-only)"""
+    orguser: OrgUser = request.orguser
+
+    try:
+        comment = CommentService.update_comment(
+            comment_id=comment_id,
+            org=orguser.org,
+            orguser=orguser,
+            content=payload.content,
+            mentioned_emails=payload.mentioned_emails,
+        )
+        return api_response(
+            success=True,
+            data=CommentResponse.from_model(comment),
+            message="Comment updated",
+        )
+    except CommentNotFoundError as err:
+        raise HttpError(404, str(err)) from err
+    except CommentPermissionError as err:
+        raise HttpError(403, str(err)) from err
+
+
+@report_router.delete("/{snapshot_id}/comments/{comment_id}/", response=ApiResponse)
+@has_permission(["can_view_dashboards"])
+def delete_comment(request, snapshot_id: int, comment_id: int):
+    """Delete a comment (author-only)"""
+    orguser: OrgUser = request.orguser
+
+    try:
+        CommentService.delete_comment(
+            comment_id=comment_id,
+            org=orguser.org,
+            orguser=orguser,
+        )
+        return api_response(success=True, message="Comment deleted")
+    except CommentNotFoundError as err:
+        raise HttpError(404, str(err)) from err
+    except CommentPermissionError as err:
+        raise HttpError(403, str(err)) from err