diff --git a/src/sentry/integrations/slack/action_endpoint.py b/src/sentry/integrations/slack/action_endpoint.py index cc400f7e0a8062..5c987d794fa927 100644 --- a/src/sentry/integrations/slack/action_endpoint.py +++ b/src/sentry/integrations/slack/action_endpoint.py @@ -31,8 +31,11 @@ class SlackActionEndpoint(Endpoint): authentication_classes = () permission_classes = () - def api_error(self, error): - logger.info("slack.action.api-error", extra={"response": error.body}) + def api_error(self, error, action_type, logging_data): + logging_data = logging_data.copy() + logging_data["response"] = error.body + logging_data["action_type"] = action_type + logger.info("slack.action.api-error", extra=logging_data) return self.respond( { "response_type": "ephemeral", @@ -155,11 +158,15 @@ def post(self, request): channel_id = data.get("channel", {}).get("id") user_id = data.get("user", {}).get("id") + logging_data["channel_id"] = channel_id + logging_data["user_id"] = user_id + integration = slack_request.integration logging_data["integration_id"] = integration.id # Determine the issue group action is being taken on group_id = slack_request.callback_data["issue"] + logging_data["group_id"] = group_id # Actions list may be empty when receiving a dialog response action_list = data.get("actions", []) @@ -175,6 +182,8 @@ def post(self, request): logger.error("slack.action.invalid-issue", extra=logging_data) return self.respond(status=403) + logging_data["organization_id"] = group.organization.id + # Determine the acting user by slack identity try: idp = IdentityProvider.objects.get(type="slack", external_id=slack_request.team_id) @@ -205,7 +214,7 @@ def post(self, request): try: self.on_status(request, identity, group, action, data, integration) except client.ApiError as e: - return self.api_error(e) + return self.api_error(e, "status_dialog", logging_data) group = Group.objects.get(id=group.id) attachment = build_group_attachment(group, identity=identity, actions=[action]) @@ -230,6 +239,7 @@ def post(self, request): defer_attachment_update = False # Handle interaction actions + action_type = None try: for action in action_list: action_type = action["name"] @@ -242,7 +252,7 @@ def post(self, request): self.open_resolve_dialog(data, group, integration) defer_attachment_update = True except client.ApiError as e: - return self.api_error(e) + return self.api_error(e, action_type, logging_data) if defer_attachment_update: return self.respond() diff --git a/tests/sentry/integrations/slack/test_action_endpoint.py b/tests/sentry/integrations/slack/test_action_endpoint.py index f20b93b804f756..868ed6c86fa756 100644 --- a/tests/sentry/integrations/slack/test_action_endpoint.py +++ b/tests/sentry/integrations/slack/test_action_endpoint.py @@ -5,6 +5,7 @@ from six.moves.urllib.parse import parse_qs from mock import patch +from sentry.api import client from sentry import options from sentry.models import ( Integration, @@ -335,6 +336,61 @@ def test_permission_denied(self): assert not resp.data["replace_original"] assert resp.data["text"] == "Sentry can't perform that action right now on your behalf!" + @responses.activate + @patch("sentry.api.client.put") + def test_handle_submission_fail(self, client_put): + status_action = {"name": "resolve_dialog", "value": "resolve_dialog"} + + # Expect request to open dialog on slack + responses.add( + method=responses.POST, + url="https://slack.com/api/dialog.open", + body='{"ok": true}', + status=200, + content_type="application/json", + ) + + resp = self.post_webhook(action_data=[status_action]) + assert resp.status_code == 200, resp.content + + # Opening dialog should *not* cause the current message to be updated + assert resp.content == "" + + data = parse_qs(responses.calls[0].request.body) + assert data["token"][0] == self.integration.metadata["access_token"] + assert data["trigger_id"][0] == self.trigger_id + assert "dialog" in data + + dialog = json.loads(data["dialog"][0]) + callback_data = json.loads(dialog["callback_id"]) + assert int(callback_data["issue"]) == self.group1.id + assert callback_data["orig_response_url"] == self.response_url + + # Completing the dialog will update the message + responses.add( + method=responses.POST, + url=self.response_url, + body='{"ok": true}', + status=200, + content_type="application/json", + ) + + # make the client raise an API error + client_put.side_effect = client.ApiError( + 403, '{"detail":"You do not have permission to perform this action."}' + ) + + resp = self.post_webhook( + type="dialog_submission", + callback_id=dialog["callback_id"], + data={"submission": {"resolve_type": "resolved"}}, + ) + + client_put.asser_called() + + assert resp.status_code == 200, resp.content + assert resp.data["text"] == "Sentry can't perform that action right now on your behalf!" + def test_invalid_token(self): resp = self.post_webhook(token="invalid") assert resp.status_code == 401