Skip to content

Commit 5541f9f

Browse files
committed
fix(alerts): Make clear that superuser may edit comments
1 parent 6962389 commit 5541f9f

File tree

3 files changed

+69
-4
lines changed

3 files changed

+69
-4
lines changed

src/sentry/api/endpoints/organization_incident_comment_details.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,15 @@ def convert_args(self, request, activity_id, *args, **kwargs):
2828
args, kwargs = super(CommentDetailsEndpoint, self).convert_args(request, *args, **kwargs)
2929

3030
try:
31+
# Superusers may mutate any comment
32+
user_filter = {} if request.user.is_superuser else {"user": request.user}
33+
3134
kwargs["activity"] = IncidentActivity.objects.get(
3235
id=activity_id,
33-
user=request.user,
3436
incident=kwargs["incident"],
3537
# Only allow modifying comments
3638
type=IncidentActivityType.COMMENT.value,
39+
**user_filter
3740
)
3841
except IncidentActivity.DoesNotExist:
3942
raise ResourceDoesNotExist

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import ConfigStore from 'app/stores/configStore';
77
import LinkWithConfirmation from 'app/components/links/linkWithConfirmation';
88
import {User} from 'app/types';
99
import {Theme} from 'app/utils/theme';
10+
import Tooltip from 'app/components/tooltip';
1011

1112
import EditorTools from './editorTools';
1213

@@ -17,17 +18,23 @@ type Props = {
1718
onDelete: () => void;
1819
};
1920

20-
function canEdit(editingUser: User) {
21+
function canEdit(commentUser: User) {
2122
const user = ConfigStore.get('user');
22-
return user && (user.isSuperuser || user.id === editingUser.id);
23+
24+
return user && (user.isSuperuser || user.id === commentUser.id);
2325
}
2426

2527
const NoteHeader = ({authorName, user, onEdit, onDelete}: Props) => (
2628
<div>
2729
<ActivityAuthor>{authorName}</ActivityAuthor>
2830
{canEdit(user) && (
2931
<EditorTools>
30-
<Edit onClick={onEdit}>{t('Edit')}</Edit>
32+
<Tooltip
33+
title={t('You can edit this comment due to your superuser status')}
34+
disabled={!ConfigStore.get('user').isSuperuser}
35+
>
36+
<Edit onClick={onEdit}>{t('Edit')}</Edit>
37+
</Tooltip>
3138
<LinkWithConfirmation
3239
title={t('Remove')}
3340
message={t('Are you sure you wish to delete this comment?')}

tests/sentry/api/endpoints/test_organization_incident_comment_details.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ def setUp(self):
1919
self.incident, user=self.user, type=IncidentActivityType.DETECTED.value
2020
)
2121

22+
user2 = self.create_user()
23+
self.user2_activity = self.create_incident_comment(
24+
incident=self.incident, user=user2, comment="hello from another user"
25+
)
26+
2227
@fixture
2328
def organization(self):
2429
return self.create_organization()
@@ -76,6 +81,34 @@ def test_simple(self):
7681
assert activity.user == self.user
7782
assert activity.comment == comment
7883

84+
def test_cannot_edit_others_comment(self):
85+
with self.feature("organizations:incidents"):
86+
self.get_valid_response(
87+
self.organization.slug,
88+
self.incident.identifier,
89+
self.user2_activity.id,
90+
comment="edited comment",
91+
status_code=404,
92+
)
93+
94+
def test_superuser_can_edit(self):
95+
self.user.is_superuser = True
96+
self.user.save()
97+
98+
edited_comment = "this comment has been edited"
99+
100+
with self.feature("organizations:incidents"):
101+
self.get_valid_response(
102+
self.organization.slug,
103+
self.incident.identifier,
104+
self.user2_activity.id,
105+
comment=edited_comment,
106+
status_code=200,
107+
)
108+
activity = IncidentActivity.objects.get(id=self.user2_activity.id)
109+
assert activity.user != self.user
110+
assert activity.comment == edited_comment
111+
79112

80113
class OrganizationIncidentCommentDeleteEndpointTest(BaseIncidentCommentDetailsTest, APITestCase):
81114
method = "delete"
@@ -86,3 +119,25 @@ def test_simple(self):
86119
self.organization.slug, self.incident.identifier, self.activity.id, status_code=204
87120
)
88121
assert not IncidentActivity.objects.filter(id=self.activity.id).exists()
122+
123+
def test_cannot_delete_others_comments(self):
124+
with self.feature("organizations:incidents"):
125+
self.get_valid_response(
126+
self.organization.slug,
127+
self.incident.identifier,
128+
self.user2_activity.id,
129+
status_code=404,
130+
)
131+
132+
def test_superuser_can_delete(self):
133+
self.user.is_superuser = True
134+
self.user.save()
135+
136+
with self.feature("organizations:incidents"):
137+
self.get_valid_response(
138+
self.organization.slug,
139+
self.incident.identifier,
140+
self.user2_activity.id,
141+
status_code=204,
142+
)
143+
assert not IncidentActivity.objects.filter(id=self.user2_activity.id).exists()

0 commit comments

Comments
 (0)