Browse Source

fix(slack): Remove `organization` from `build_linking_url()` (#29299)

Marcos Gaeta 3 years ago
parent
commit
ae7d0c40b0

+ 1 - 1
src/sentry/api/event_search.py

@@ -682,7 +682,7 @@ class SearchVisitor(NodeVisitor):
                 # minutes). So we fall through to numeric if it's not a
                 # duration key
                 #
-                # TODO(epurkhsier): Should we validate that the field is
+                # TODO(epurkhiser): Should we validate that the field is
                 # numeric and do some other fallback if it's not?
                 aggregate_value = parse_numeric_value(*search_value)
         except ValueError:

+ 17 - 8
src/sentry/api/helpers/group_index.py

@@ -73,6 +73,11 @@ from sentry.utils.hashlib import md5_text
 
 delete_logger = logging.getLogger("sentry.deletions.api")
 
+# Bulk mutations are limited to 1000 items.
+# TODO(dcramer): It'd be nice to support more than this, but it's a bit too
+#  complicated right now.
+BULK_MUTATION_LIMIT = 1000
+
 
 class ValidationError(Exception):
     pass
@@ -413,10 +418,12 @@ def delete_groups(request, projects, organization_id, search_fn):
         )
     else:
         try:
-            # bulk mutations are limited to 1000 items
-            # TODO(dcramer): it'd be nice to support more than this, but its
-            # a bit too complicated right now
-            cursor_result, _ = search_fn({"limit": 1000, "paginator_options": {"max_limit": 1000}})
+            cursor_result, _ = search_fn(
+                {
+                    "limit": BULK_MUTATION_LIMIT,
+                    "paginator_options": {"max_limit": BULK_MUTATION_LIMIT},
+                }
+            )
         except ValidationError as exc:
             return Response({"detail": str(exc)}, status=400)
 
@@ -591,10 +598,12 @@ def update_groups(request, group_ids, projects, organization_id, search_fn):
 
     if not group_ids:
         try:
-            # bulk mutations are limited to 1000 items
-            # TODO(dcramer): it'd be nice to support more than this, but its
-            # a bit too complicated right now
-            cursor_result, _ = search_fn({"limit": 1000, "paginator_options": {"max_limit": 1000}})
+            cursor_result, _ = search_fn(
+                {
+                    "limit": BULK_MUTATION_LIMIT,
+                    "paginator_options": {"max_limit": BULK_MUTATION_LIMIT},
+                }
+            )
         except ValidationError as exc:
             return Response({"detail": str(exc)}, status=400)
 

+ 1 - 3
src/sentry/integrations/slack/endpoints/action.py

@@ -241,9 +241,7 @@ class SlackActionEndpoint(Endpoint):  # type: ignore
             return self.respond(status=403)
 
         if not identity:
-            associate_url = build_linking_url(
-                integration, group.organization, user_id, channel_id, response_url
-            )
+            associate_url = build_linking_url(integration, user_id, channel_id, response_url)
             return self.respond(
                 {
                     "response_type": "ephemeral",

+ 1 - 4
src/sentry/integrations/slack/endpoints/base.py

@@ -60,11 +60,8 @@ class SlackDMEndpoint(Endpoint, abc.ABC):  # type: ignore
                 slack_request, ALREADY_LINKED_MESSAGE.format(username=slack_request.identity_str)
             )
 
-        integration = slack_request.integration
-        organization = integration.organizations.all()[0]
         associate_url = build_linking_url(
-            integration=integration,
-            organization=organization,
+            integration=slack_request.integration,
             slack_id=slack_request.user_id,
             channel_id=slack_request.channel_id,
             response_url=slack_request.response_url,

+ 0 - 7
src/sentry/integrations/slack/endpoints/event.py

@@ -79,15 +79,8 @@ class SlackEventEndpoint(SlackDMEndpoint):  # type: ignore
         slack_request: SlackRequest,
         integration: Integration,
     ):
-        # This will break if multiple Sentry orgs are added
-        # to a single Slack workspace and a user is a part of one
-        # org and not the other. Since we pick the first org
-        # in the integration organizations set, we might be picking
-        # the org the user is not a part of.
-        organization = integration.organizations.all()[0]
         associate_url = build_linking_url(
             integration=integration,
-            organization=organization,
             slack_id=slack_request.user_id,
             channel_id=slack_request.channel_id,
             response_url=slack_request.response_url,

+ 2 - 8
src/sentry/integrations/slack/views/link_identity.py

@@ -5,7 +5,7 @@ from rest_framework.request import Request
 from rest_framework.response import Response
 
 from sentry.integrations.utils import get_identity_or_404
-from sentry.models import Identity, IdentityStatus, Integration, Organization
+from sentry.models import Identity, IdentityStatus, Integration
 from sentry.types.integrations import ExternalProviders
 from sentry.utils.signing import unsign
 from sentry.web.decorators import transaction_start
@@ -22,16 +22,11 @@ SUCCESS_LINKED_MESSAGE = (
 
 
 def build_linking_url(
-    integration: Integration,
-    organization: Organization,
-    slack_id: str,
-    channel_id: str,
-    response_url: str,
+    integration: Integration, slack_id: str, channel_id: str, response_url: str
 ) -> str:
     return base_build_linking_url(
         "sentry-integration-slack-link-identity",
         integration_id=integration.id,
-        organization_id=organization.id,
         slack_id=slack_id,
         channel_id=channel_id,
         response_url=response_url,
@@ -54,7 +49,6 @@ class SlackLinkIdentityView(BaseView):  # type: ignore
             ExternalProviders.SLACK,
             request.user,
             integration_id=params["integration_id"],
-            organization_id=params["organization_id"],
         )
 
         if request.method != "POST":

+ 1 - 1
static/app/actionCreators/indicator.tsx

@@ -106,7 +106,7 @@ const prettyFormString = (val: ChangeValue, model: FormModel, fieldName: string)
 
   if (descriptor && typeof descriptor.formatMessageValue === 'function') {
     const initialData = model.initialData;
-    // XXX(epurkhsier): We pass the "props" as the descriptor and initialData.
+    // XXX(epurkhiser): We pass the "props" as the descriptor and initialData.
     // This isn't necessarily all of the props of the form field, but should
     // make up a good portion needed for formatting.
     return descriptor.formatMessageValue(val, {...descriptor, initialData});

+ 0 - 1
tests/sentry/incidents/endpoints/test_project_alert_rule_index.py

@@ -220,7 +220,6 @@ class AlertRuleCreateEndpointTest(APITestCase):
         alert_rule = AlertRule.objects.get(id=resp.data["id"])
         assert resp.data == serialize(alert_rule, self.user)
 
-    # TODO MARCOS
     @patch(
         "sentry.integrations.slack.utils.channel.get_channel_id_with_timeout",
         side_effect=[("#", 10, False), ("#", 10, False), ("#", 20, False)],

+ 1 - 1
tests/sentry/integrations/slack/endpoints/test_action.py

@@ -94,7 +94,7 @@ class StatusActionTest(BaseEventTest):
         resp = self.post_webhook(slack_user={"id": "invalid-id", "domain": "example"})
 
         associate_url = build_linking_url(
-            self.integration, self.organization, "invalid-id", "C065W1189", self.response_url
+            self.integration, "invalid-id", "C065W1189", self.response_url
         )
 
         assert resp.status_code == 200, resp.content

+ 1 - 5
tests/sentry/integrations/slack/endpoints/test_commands.py

@@ -171,11 +171,7 @@ class SlackCommandsLinkUserTest(SlackCommandsTest):
         assert not self.find_identity()
 
         linking_url = build_linking_url(
-            self.integration,
-            self.organization,
-            "UXXXXXXX1",
-            "CXXXXXXX9",
-            "http://example.slack.com/response_url",
+            self.integration, "UXXXXXXX1", "CXXXXXXX9", "http://example.slack.com/response_url"
         )
 
         response = self.client.get(linking_url)

Some files were not shown because too many files changed in this diff