Skip to content

Commit

Permalink
Make Slack connection error more descriptive (#5007)
Browse files Browse the repository at this point in the history
# What this PR does

<img width="517" alt="Screenshot 2024-09-10 at 17 11 51"
src="https://github.com/user-attachments/assets/086eb4ae-6ea3-467c-a1ed-6d6f51ddc3e7">


<!--
*Note*: If you want the issue to be auto-closed once the PR is merged,
change "Related to" to "Closes" in the line above.
If you have more than one GitHub issue that this PR closes, be sure to
preface
each issue link with a [closing
keyword](https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests#linking-a-pull-request-to-an-issue).
This ensures that the issue(s) are auto-closed once the PR has been
merged.
-->

## Checklist

- [x] Unit, integration, and e2e (if applicable) tests updated
- [x] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] Added the relevant release notes label (see labels prefixed w/
`release:`). These labels dictate how your PR will
    show up in the autogenerated release notes.
  • Loading branch information
vadimkerr authored Sep 10, 2024
1 parent d1cb862 commit e287dec
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 5 deletions.
32 changes: 32 additions & 0 deletions engine/apps/slack/tests/test_interactive_api_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,38 @@ def slack_team_identity(make_slack_team_identity):
)


@patch("apps.slack.views.SlackEventApiEndpointView.verify_signature", return_value=True)
@patch("apps.slack.views.SlackEventApiEndpointView._open_warning_window_if_needed")
@pytest.mark.django_db
def test_no_user_in_organization_for_slack_team_identity(
mock_open_warning_window_if_needed,
_mock_verify_signature,
make_organization,
make_slack_user_identity,
slack_team_identity,
):
# only create SlackUserIdentity, not actual OnCall user
make_slack_user_identity(slack_team_identity=slack_team_identity, slack_id=SLACK_USER_ID)
organization = make_organization(slack_team_identity=slack_team_identity, grafana_url="https://test.com")

event_payload = {
"type": PayloadType.BLOCK_ACTIONS,
"trigger_id": EVENT_TRIGGER_ID,
"user": {"id": SLACK_USER_ID},
"team": {"id": SLACK_TEAM_ID},
"actions": [{"value": json.dumps({"organization_id": organization.id})}],
}

response = _make_request(event_payload)
assert response.status_code == status.HTTP_200_OK

mock_open_warning_window_if_needed.assert_called_once_with(
event_payload,
slack_team_identity,
"Permission denied. Please connect your Slack account to OnCall: https://test.com/a/grafana-oncall-app/users/me/",
)


@patch("apps.slack.views.SlackEventApiEndpointView.verify_signature", return_value=True)
@patch("apps.slack.views.SlackEventApiEndpointView._open_warning_window_if_needed")
@pytest.mark.django_db
Expand Down
13 changes: 8 additions & 5 deletions engine/apps/slack/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import json
import logging
from contextlib import suppress
from urllib.parse import urljoin

from django.conf import settings
from django.core.exceptions import ObjectDoesNotExist
Expand Down Expand Up @@ -291,11 +292,13 @@ def post(self, request):
return Response()
elif organization:
user = slack_user_identity.get_user(organization)
if not user:
# Means that user slack_user_identity is not in any organization, connected to this Slack workspace
warning_text = "Permission denied. Please connect your Slack account to OnCall."
# Open pop-up to inform user why OnCall bot doesn't work if any action was triggered
self._open_warning_window_if_needed(payload, slack_team_identity, warning_text)
if not user: # SlackUserIdentity exists but not connected to any user in this organization
user_settings_url = urljoin(organization.grafana_url, "/a/grafana-oncall-app/users/me/")
self._open_warning_window_if_needed(
payload,
slack_team_identity,
f"Permission denied. Please connect your Slack account to OnCall: {user_settings_url}",
)
return Response(status=200)
# direct paging / manual incident / schedule update dialogs don't require organization to be set
elif (
Expand Down

0 comments on commit e287dec

Please sign in to comment.