fix(alerts): Make clear that superuser may edit comments (#18455)

Author: evanpurkhiser

src/sentry/api/endpoints/organization_incident_comment_details.py

@@ -28,12 +28,15 @@ def convert_args(self, request, activity_id, *args, **kwargs):
         args, kwargs = super(CommentDetailsEndpoint, self).convert_args(request, *args, **kwargs)
 
         try:
+            # Superusers may mutate any comment
+            user_filter = {} if request.user.is_superuser else {"user": request.user}
+
             kwargs["activity"] = IncidentActivity.objects.get(
                 id=activity_id,
-                user=request.user,
                 incident=kwargs["incident"],
                 # Only allow modifying comments
                 type=IncidentActivityType.COMMENT.value,
+                **user_filter
             )
         except IncidentActivity.DoesNotExist:
             raise ResourceDoesNotExist

src/sentry/static/sentry/app/components/activity/note/header.tsx

@@ -7,6 +7,7 @@ import ConfigStore from 'app/stores/configStore';
 import LinkWithConfirmation from 'app/components/links/linkWithConfirmation';
 import {User} from 'app/types';
 import {Theme} from 'app/utils/theme';
+import Tooltip from 'app/components/tooltip';
 
 import EditorTools from './editorTools';
 
@@ -17,28 +18,38 @@ type Props = {
   onDelete: () => void;
 };
 
-function canEdit(editingUser: User) {
-  const user = ConfigStore.get('user');
-  return user && (user.isSuperuser || user.id === editingUser.id);
-}
+const NoteHeader = ({authorName, user, onEdit, onDelete}: Props) => {
+  const activeUser = ConfigStore.get('user');
+  const canEdit = activeUser && (activeUser.isSuperuser || user.id === activeUser.id);
 
-const NoteHeader = ({authorName, user, onEdit, onDelete}: Props) => (
-  <div>
-    <ActivityAuthor>{authorName}</ActivityAuthor>
-    {canEdit(user) && (
-      <EditorTools>
-        <Edit onClick={onEdit}>{t('Edit')}</Edit>
-        <LinkWithConfirmation
-          title={t('Remove')}
-          message={t('Are you sure you wish to delete this comment?')}
-          onConfirm={onDelete}
-        >
-          <Remove>{t('Remove')}</Remove>
-        </LinkWithConfirmation>
-      </EditorTools>
-    )}
-  </div>
-);
+  return (
+    <div>
+      <ActivityAuthor>{authorName}</ActivityAuthor>
+      {canEdit && (
+        <EditorTools>
+          <Tooltip
+            title={t('You can edit this comment due to your superuser status')}
+            disabled={!activeUser.isSuperuser}
+          >
+            <Edit onClick={onEdit}>{t('Edit')}</Edit>
+          </Tooltip>
+          <Tooltip
+            title={t('You can delete this comment due to your superuser status')}
+            disabled={!activeUser.isSuperuser}
+          >
+            <LinkWithConfirmation
+              title={t('Remove')}
+              message={t('Are you sure you wish to delete this comment?')}
+              onConfirm={onDelete}
+            >
+              <Remove>{t('Remove')}</Remove>
+            </LinkWithConfirmation>
+          </Tooltip>
+        </EditorTools>
+      )}
+    </div>
+  );
+};
 
 const getActionStyle = (p: {theme: Theme}) => `
   padding: 0 7px;

tests/sentry/api/endpoints/test_organization_incident_comment_details.py

@@ -19,6 +19,11 @@ def setUp(self):
             self.incident, user=self.user, type=IncidentActivityType.DETECTED.value
         )
 
+        user2 = self.create_user()
+        self.user2_activity = self.create_incident_comment(
+            incident=self.incident, user=user2, comment="hello from another user"
+        )
+
     @fixture
     def organization(self):
         return self.create_organization()
@@ -76,6 +81,34 @@ def test_simple(self):
         assert activity.user == self.user
         assert activity.comment == comment
 
+    def test_cannot_edit_others_comment(self):
+        with self.feature("organizations:incidents"):
+            self.get_valid_response(
+                self.organization.slug,
+                self.incident.identifier,
+                self.user2_activity.id,
+                comment="edited comment",
+                status_code=404,
+            )
+
+    def test_superuser_can_edit(self):
+        self.user.is_superuser = True
+        self.user.save()
+
+        edited_comment = "this comment has been edited"
+
+        with self.feature("organizations:incidents"):
+            self.get_valid_response(
+                self.organization.slug,
+                self.incident.identifier,
+                self.user2_activity.id,
+                comment=edited_comment,
+                status_code=200,
+            )
+        activity = IncidentActivity.objects.get(id=self.user2_activity.id)
+        assert activity.user != self.user
+        assert activity.comment == edited_comment
+
 
 class OrganizationIncidentCommentDeleteEndpointTest(BaseIncidentCommentDetailsTest, APITestCase):
     method = "delete"
@@ -86,3 +119,25 @@ def test_simple(self):
                 self.organization.slug, self.incident.identifier, self.activity.id, status_code=204
             )
         assert not IncidentActivity.objects.filter(id=self.activity.id).exists()
+
+    def test_cannot_delete_others_comments(self):
+        with self.feature("organizations:incidents"):
+            self.get_valid_response(
+                self.organization.slug,
+                self.incident.identifier,
+                self.user2_activity.id,
+                status_code=404,
+            )
+
+    def test_superuser_can_delete(self):
+        self.user.is_superuser = True
+        self.user.save()
+
+        with self.feature("organizations:incidents"):
+            self.get_valid_response(
+                self.organization.slug,
+                self.incident.identifier,
+                self.user2_activity.id,
+                status_code=204,
+            )
+        assert not IncidentActivity.objects.filter(id=self.user2_activity.id).exists()