Merge remote-tracking branch 'origin/main' into codex/pratiksha-chat-with-dashboards-impl

Author: Ishankoradia

ddpui/core/reports/comment_service.py

@@ -166,18 +166,37 @@ def delete_comment(
         org: Org,
         orguser: OrgUser,
     ) -> None:
-        """Soft-delete a comment. Author-only."""
+        """Delete a comment. Author-only.
+
+        Hard-deletes if no other user has commented in the thread (same
+        snapshot + target_type + chart_id). Soft-deletes otherwise so the
+        "This message was deleted" placeholder is shown alongside others'
+        comments.
+        """
         comment = CommentService._get_comment(comment_id, org)
 
         if comment.author != orguser:
             raise CommentPermissionError("You can only delete your own comments")
 
-        comment.is_deleted = True
-        comment.content = ""
-        comment.mentioned_emails = []
-        comment.save()
-
-        logger.info(f"Soft-deleted comment {comment_id}")
+        # Check if another author has commented in this thread
+        thread_query = Q(
+            snapshot=comment.snapshot,
+            target_type=comment.target_type,
+        )
+        if comment.target_type == CommentTargetType.CHART:
+            thread_query &= Q(snapshot_chart_id=comment.snapshot_chart_id)
+
+        has_other_authors = Comment.objects.filter(thread_query).exclude(author=orguser).exists()
+
+        if has_other_authors:
+            comment.is_deleted = True
+            comment.content = ""
+            comment.mentioned_emails = []
+            comment.save()
+            logger.info(f"Soft-deleted comment {comment_id}")
+        else:
+            comment.delete()
+            logger.info(f"Hard-deleted comment {comment_id}")
 
     # language=SQL
     _COMMENT_STATES_SQL = """

ddpui/tests/api_tests/test_comment_api.py

@@ -494,14 +494,12 @@ def test_delete_own(self, mock_mentions, orguser, snapshot, org):
             author=orguser,
             org=org,
         )
+        comment_id = comment.id
         request = mock_request(orguser)
-        response = delete_comment(request, snapshot.id, comment.id)
+        response = delete_comment(request, snapshot.id, comment_id)
         assert response["success"] is True
-        comment.refresh_from_db()
-        assert comment.is_deleted is True
-        assert comment.content == ""
-        assert comment.mentioned_emails == []
-        comment.delete()
+        # sole author in thread => hard-delete
+        assert not Comment.objects.filter(id=comment_id).exists()
 
     def test_delete_other_forbidden(self, orguser, other_orguser, snapshot, org):
         comment = Comment.objects.create(

ddpui/tests/core/reports/test_comment_service_mutations.py

@@ -209,7 +209,8 @@ def test_not_found_raises(self, org, author_orguser):
 class TestDeleteComment:
     """Tests for CommentService.delete_comment"""
 
-    def test_success(self, snapshot, author_orguser, org):
+    def test_hard_deletes_sole_comment(self, snapshot, author_orguser, org):
+        """Only comment in thread, only author — hard-delete."""
         comment = Comment.objects.create(
             target_type=CommentTargetType.SUMMARY,
             snapshot=snapshot,
@@ -218,16 +219,67 @@ def test_success(self, snapshot, author_orguser, org):
             author=author_orguser,
             org=org,
         )
+        comment_id = comment.id
         CommentService.delete_comment(
-            comment_id=comment.id,
+            comment_id=comment_id,
             org=org,
             orguser=author_orguser,
         )
-        comment.refresh_from_db()
-        assert comment.is_deleted is True
-        assert comment.content == ""
-        assert comment.mentioned_emails == []
-        comment.delete()
+        assert not Comment.objects.filter(id=comment_id).exists()
+
+    def test_hard_deletes_multiple_own_comments(self, snapshot, author_orguser, org):
+        """Multiple comments in thread but ALL by the same author — hard-delete."""
+        c1 = Comment.objects.create(
+            target_type=CommentTargetType.SUMMARY,
+            snapshot=snapshot,
+            content="My first",
+            author=author_orguser,
+            org=org,
+        )
+        c2 = Comment.objects.create(
+            target_type=CommentTargetType.SUMMARY,
+            snapshot=snapshot,
+            content="My second",
+            author=author_orguser,
+            org=org,
+        )
+        c2_id = c2.id
+        CommentService.delete_comment(
+            comment_id=c2_id,
+            org=org,
+            orguser=author_orguser,
+        )
+        assert not Comment.objects.filter(id=c2_id).exists()
+        c1.delete()
+
+    def test_soft_deletes_when_other_author_exists(
+        self, snapshot, author_orguser, other_orguser, org
+    ):
+        """Another user has commented in the thread — soft-delete."""
+        Comment.objects.create(
+            target_type=CommentTargetType.SUMMARY,
+            snapshot=snapshot,
+            content="Other person's comment",
+            author=other_orguser,
+            org=org,
+        )
+        my_comment = Comment.objects.create(
+            target_type=CommentTargetType.SUMMARY,
+            snapshot=snapshot,
+            content="Delete me",
+            mentioned_emails=["someone@test.com"],
+            author=author_orguser,
+            org=org,
+        )
+        CommentService.delete_comment(
+            comment_id=my_comment.id,
+            org=org,
+            orguser=author_orguser,
+        )
+        my_comment.refresh_from_db()
+        assert my_comment.is_deleted is True
+        assert my_comment.content == ""
+        assert my_comment.mentioned_emails == []
 
     def test_non_author_raises(self, snapshot, author_orguser, other_orguser, org):
         comment = Comment.objects.create(

ddpui/websockets/__init__.py

@@ -5,7 +5,7 @@
 from urllib.parse import parse_qs
 from django.contrib.auth.models import User
 
-from ddpui.websockets.schemas import WebsocketResponse
+from ddpui.websockets.schemas import WebsocketResponse, WebsocketCloseCodes
 from ddpui.models.org_user import OrgUser
 from ddpui.utils.custom_logger import CustomLogger
 
@@ -72,14 +72,34 @@ def authenticate_user(self, token: str, orgslug: str):
     def respond(self, message: WebsocketResponse):
         self.send(text_data=json.dumps(message.model_dump()))
 
+    def _get_cookie(self, name: str) -> str | None:
+        """Extract a cookie value from the WebSocket scope headers."""
+        for header_name, header_value in self.scope.get("headers", []):
+            if header_name == b"cookie":
+                cookie = SimpleCookie(header_value.decode())
+                if name in cookie:
+                    return cookie[name].value
+        return None
+
     def connect(self):
         query_string = parse_qs(self.scope["query_string"].decode())
-        token = query_string.get("token", [None])[0]
         orgslug = query_string.get("orgslug", [None])[0]
 
-        if self.authenticate_user(token, orgslug):
-            logger.info("User authenticated, establishing connection")
+        # Read JWT from the access_token httpOnly cookie (webapp_v2)
+        token = self._get_cookie("access_token")
+
+        # TODO: remove this fallback once webapp_v1 is fully deprecated
+        if not token:
+            token = query_string.get("token", [None])[0]
+
+        if not token:
+            logger.info("No access_token cookie found, closing connection")
+            self.accept()
+            self.close(code=WebsocketCloseCodes.NO_TOKEN)
+        elif not self.authenticate_user(token, orgslug):
+            logger.info("Authentication failed (invalid/expired token), closing connection")
             self.accept()
+            self.close(code=WebsocketCloseCodes.INVALID_TOKEN)
         else:
-            logger.info("Authentication failed, closing connection")
-            self.close()
+            logger.info("User authenticated via cookie, establishing connection")
+            self.accept()

ddpui/websockets/schemas.py

@@ -3,6 +3,13 @@
 from enum import Enum
 
 
+class WebsocketCloseCodes:
+    """Custom WebSocket close codes (4000-4999 range is for application use)"""
+
+    NO_TOKEN = 4001
+    INVALID_TOKEN = 4003
+
+
 class WebsocketResponseStatus(str, Enum):
     SUCCESS = "success"
     ERROR = "error"