Skip to content

Commit 77b376e

Browse files
authored
🐛 fix(slack): member invite messages (#92440)
1 parent 8d0b9b3 commit 77b376e

File tree

6 files changed

+54
-43
lines changed

6 files changed

+54
-43
lines changed

src/sentry/integrations/slack/message_builder/notifications/base.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ def build(self) -> SlackBlock:
3434
self.recipient, ExternalProviders.SLACK
3535
)
3636
actions = self.notification.get_message_actions(self.recipient, ExternalProviders.SLACK)
37-
callback_id = orjson.dumps(callback_id_raw).decode() if callback_id_raw else None
37+
block_id = orjson.dumps(callback_id_raw).decode() if callback_id_raw else None
3838

3939
first_block_text = ""
4040
if title_link:
@@ -61,6 +61,4 @@ def build(self) -> SlackBlock:
6161
if actions_block:
6262
blocks.append({"type": "actions", "elements": [action for action in actions_block]})
6363

64-
return self._build_blocks(
65-
*blocks, fallback_text=text if text else None, callback_id=callback_id
66-
)
64+
return self._build_blocks(*blocks, fallback_text=text if text else None, block_id=block_id)

src/sentry/integrations/slack/webhooks/action.py

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -675,8 +675,14 @@ def handle_enable_notifications(self, slack_request: SlackActionRequest) -> Resp
675675
def handle_member_approval(self, slack_request: SlackActionRequest, action: str) -> Response:
676676
identity_user = slack_request.get_identity_user()
677677

678+
response_url = slack_request.data["response_url"]
679+
webhook_client = WebhookClient(response_url)
680+
678681
if not identity_user:
679-
return self.respond_with_text(NO_IDENTITY_MESSAGE)
682+
webhook_client.send(
683+
text=NO_IDENTITY_MESSAGE, response_type="in_channel", replace_original=False
684+
)
685+
return self.respond()
680686

681687
member_id = slack_request.callback_data["member_id"]
682688

@@ -685,27 +691,41 @@ def handle_member_approval(self, slack_request: SlackActionRequest, action: str)
685691
except OrganizationMember.DoesNotExist:
686692
# member request is gone, likely someone else rejected it
687693
member_email = slack_request.callback_data["member_email"]
688-
return self.respond_with_text(f"Member invitation for {member_email} no longer exists.")
694+
webhook_client.send(
695+
text=f"Member invitation for {member_email} no longer exists.",
696+
response_type="in_channel",
697+
replace_original=False,
698+
)
699+
return self.respond()
689700

690701
organization = member.organization
691702

692703
if not organization.has_access(identity_user):
693-
return self.respond_with_text(NO_ACCESS_MESSAGE)
704+
webhook_client.send(
705+
text=NO_ACCESS_MESSAGE,
706+
response_type="in_channel",
707+
replace_original=False,
708+
)
709+
return self.respond()
694710

695711
# row should exist because we have access
696712
member_of_approver = OrganizationMember.objects.get(
697713
user_id=identity_user.id, organization=organization
698714
)
699715
access = from_member(member_of_approver)
700716
if not access.has_scope("member:admin"):
701-
return self.respond_with_text(NO_PERMISSION_MESSAGE)
717+
webhook_client.send(
718+
text=NO_PERMISSION_MESSAGE, replace_original=False, response_type="in_channel"
719+
)
720+
return self.respond()
702721

703722
# validate the org options and check against allowed_roles
704723
allowed_roles = member_of_approver.get_allowed_org_roles_to_invite()
705724
try:
706725
member.validate_invitation(identity_user, allowed_roles)
707726
except UnableToAcceptMemberInvitationException as err:
708-
return self.respond_with_text(str(err))
727+
webhook_client.send(text=str(err), replace_original=False, response_type="in_channel")
728+
return self.respond()
709729

710730
original_status = InviteStatus(member.invite_status)
711731
try:
@@ -722,7 +742,10 @@ def handle_member_approval(self, slack_request: SlackActionRequest, action: str)
722742
"member_id": member.id,
723743
},
724744
)
725-
return self.respond_ephemeral(DEFAULT_ERROR_MESSAGE)
745+
webhook_client.send(
746+
text=DEFAULT_ERROR_MESSAGE, replace_original=False, response_type="in_channel"
747+
)
748+
return self.respond()
726749

727750
if action == "approve_member":
728751
event_name = "integrations.slack.approve_member_invitation"
@@ -755,7 +778,8 @@ def handle_member_approval(self, slack_request: SlackActionRequest, action: str)
755778
verb=verb,
756779
)
757780

758-
return self.respond({"text": message})
781+
webhook_client.send(text=message, replace_original=False, response_type="in_channel")
782+
return self.respond()
759783

760784

761785
class _ModalDialog(ABC):

tests/sentry/api/endpoints/test_organization_invite_request_index.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,8 +232,10 @@ def test_request_to_invite_slack(self):
232232
== f"You are receiving this notification because you have the scope member:write | <http://testserver/settings/account/notifications/approval/?referrer=invite_request-slack-user&notification_uuid={notification_uuid}|Notification Settings>"
233233
)
234234
member = OrganizationMember.objects.get(email="eric@localhost")
235-
callback_id = orjson.loads(self.mock_post.call_args.kwargs["callback_id"])
236-
assert callback_id == {
235+
context_params = orjson.loads(
236+
orjson.loads(self.mock_post.call_args.kwargs["blocks"])[0]["block_id"]
237+
)
238+
assert context_params == {
237239
"member_id": member.id,
238240
"member_email": "eric@localhost",
239241
}

tests/sentry/api/endpoints/test_organization_join_request.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,11 +206,13 @@ def test_request_to_join_slack(self):
206206
"value": "link_clicked",
207207
},
208208
]
209-
callback_id = orjson.loads(self.mock_post.call_args.kwargs["callback_id"])
209+
context_params = orjson.loads(
210+
orjson.loads(self.mock_post.call_args.kwargs["blocks"])[0]["block_id"]
211+
)
210212

211213
with outbox_runner():
212214
member = OrganizationMember.objects.get(email=self.email)
213-
assert callback_id == {
215+
assert context_params == {
214216
"member_id": member.id,
215217
"member_email": self.email,
216218
}

tests/sentry/integrations/slack/notifications/test_nudge.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,7 @@ def test_nudge_block(self):
3535
assert blocks[1]["elements"][0]["value"] == "all_slack"
3636

3737
# Slack requires callback_id to handle enablement
38-
callback_id = orjson.loads(self.mock_post.call_args.kwargs["callback_id"])
39-
assert callback_id == {"enable_notifications": True}
38+
context_params = orjson.loads(
39+
orjson.loads(self.mock_post.call_args.kwargs["blocks"])[0]["block_id"]
40+
)
41+
assert context_params == {"enable_notifications": True}

tests/sentry/integrations/slack/webhooks/actions/test_status.py

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import orjson
44
from django.db import router
5-
from django.urls import reverse
65
from slack_sdk.errors import SlackApiError
76
from slack_sdk.models.views import View
87
from slack_sdk.web import SlackResponse
@@ -35,7 +34,6 @@
3534
from sentry.testutils.skips import requires_snuba
3635
from sentry.types.group import GroupSubStatus
3736
from sentry.users.models.identity import Identity
38-
from sentry.utils.http import absolute_uri
3937
from sentry.utils.samples import load_data
4038

4139
from . import BaseEventTest
@@ -1240,13 +1238,7 @@ def test_approve_join_request(self):
12401238
self.assert_org_member_mapping(org_member=member)
12411239
assert member.invite_status == InviteStatus.APPROVED.value
12421240

1243-
manage_url = absolute_uri(
1244-
reverse("sentry-organization-members", args=[self.organization.slug])
1245-
)
1246-
assert (
1247-
resp.data["text"]
1248-
== f"Join request for [email protected] has been approved. <{manage_url}|See Members and Requests>."
1249-
)
1241+
assert resp is not None
12501242

12511243
def test_rejected_invite_request(self):
12521244
other_user = self.create_user()
@@ -1267,13 +1259,7 @@ def test_rejected_invite_request(self):
12671259
assert resp.status_code == 200, resp.content
12681260
assert not OrganizationMember.objects.filter(id=member.id).exists()
12691261

1270-
manage_url = absolute_uri(
1271-
reverse("sentry-organization-members", args=[self.organization.slug])
1272-
)
1273-
assert (
1274-
resp.data["text"]
1275-
== f"Invite request for [email protected] has been rejected. <{manage_url}|See Members and Requests>."
1276-
)
1262+
assert resp is not None
12771263

12781264
def test_invalid_rejected_invite_request(self):
12791265
user = self.create_user(email="[email protected]")
@@ -1295,7 +1281,7 @@ def test_invalid_rejected_invite_request(self):
12951281
member.refresh_from_db()
12961282
self.assert_org_member_mapping(org_member=member)
12971283

1298-
assert resp.data["text"] == "Member invitation for [email protected] no longer exists."
1284+
assert resp is not None
12991285

13001286
def test_invitation_removed(self):
13011287
other_user = self.create_user()
@@ -1314,7 +1300,7 @@ def test_invitation_removed(self):
13141300
resp = self.post_webhook(action_data=[{"value": "approve_member"}], callback_id=callback_id)
13151301

13161302
assert resp.status_code == 200, resp.content
1317-
assert resp.data["text"] == "Member invitation for [email protected] no longer exists."
1303+
assert resp is not None
13181304

13191305
def test_invitation_already_accepted(self):
13201306
other_user = self.create_user()
@@ -1332,7 +1318,7 @@ def test_invitation_already_accepted(self):
13321318
resp = self.post_webhook(action_data=[{"value": "approve_member"}], callback_id=callback_id)
13331319

13341320
assert resp.status_code == 200, resp.content
1335-
assert resp.data["text"] == "Member invitation for [email protected] no longer exists."
1321+
assert resp is not None
13361322

13371323
def test_invitation_validation_error(self):
13381324
with unguarded_write(using=router.db_for_write(OrganizationMember)):
@@ -1352,18 +1338,15 @@ def test_invitation_validation_error(self):
13521338
resp = self.post_webhook(action_data=[{"value": "approve_member"}], callback_id=callback_id)
13531339

13541340
assert resp.status_code == 200, resp.content
1355-
assert (
1356-
resp.data["text"]
1357-
== "You do not have permission to approve a member invitation with the role owner."
1358-
)
1341+
assert resp is not None
13591342

13601343
def test_identity_not_linked(self):
13611344
with assume_test_silo_mode(SiloMode.CONTROL):
13621345
Identity.objects.filter(user=self.user).delete()
13631346
resp = self.post_webhook(action_data=[{"value": "approve_member"}], callback_id="")
13641347

13651348
assert resp.status_code == 200, resp.content
1366-
assert resp.data["text"] == "Identity not linked for user."
1349+
assert resp is not None
13671350

13681351
def test_wrong_organization(self):
13691352
other_user = self.create_user()
@@ -1382,7 +1365,7 @@ def test_wrong_organization(self):
13821365
resp = self.post_webhook(action_data=[{"value": "approve_member"}], callback_id=callback_id)
13831366

13841367
assert resp.status_code == 200, resp.content
1385-
assert resp.data["text"] == "You do not have access to the organization for the invitation."
1368+
assert resp is not None
13861369

13871370
def test_no_member_admin(self):
13881371
with unguarded_write(using=router.db_for_write(OrganizationMember)):
@@ -1403,4 +1386,4 @@ def test_no_member_admin(self):
14031386
resp = self.post_webhook(action_data=[{"value": "approve_member"}], callback_id=callback_id)
14041387

14051388
assert resp.status_code == 200, resp.content
1406-
assert resp.data["text"] == "You do not have permission to approve member invitations."
1389+
assert resp is not None

0 commit comments

Comments
 (0)