diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index fd5d107b0a33b7..d55a776caa5241 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -448,6 +448,7 @@ tests/sentry/api/endpoints/test_organization_attribute_mappings.py @get /src/sentry/sentry_apps/ @getsentry/product-owners-settings-integrations @getsentry/ecosystem /tests/sentry/sentry_apps/ @getsentry/product-owners-settings-integrations @getsentry/ecosystem /src/sentry/utils/sentry_apps/ @getsentry/ecosystem +/tests/sentry/utils/sentry_apps/ @getsentry/ecosystem /src/sentry/middleware/integrations/ @getsentry/ecosystem /src/sentry/api/endpoints/project_rule*.py @getsentry/alerts-notifications /src/sentry/api/serializers/models/rule.py @getsentry/alerts-notifications @@ -890,6 +891,7 @@ tests/sentry/api/endpoints/test_organization_attribute_mappings.py @get # Cell architecture /.agents/skills/cell-architecture @getsentry/sre-infrastructure-engineering +tests/sentry/core/endpoints/test_organization_cell.py @getsentry/sre-infrastructure-engineering # End of cell architecture # Foundational Storage diff --git a/.github/workflows/scripts/compute-sentry-selected-tests.py b/.github/workflows/scripts/compute-sentry-selected-tests.py index 7a9f2f02a4637b..65bbcd1a6ba034 100644 --- a/.github/workflows/scripts/compute-sentry-selected-tests.py +++ b/.github/workflows/scripts/compute-sentry-selected-tests.py @@ -43,6 +43,7 @@ FULL_SUITE_TRIGGERS: list[str | re.Pattern[str]] = [ "src/sentry/testutils/pytest/sentry.py", + "src/sentry/constants.py", "pyproject.toml", "src/sentry/conf/server.py", "src/sentry/web/urls.py", diff --git a/fixtures/gitlab.py b/fixtures/gitlab.py index 0b21efdc8ae780..4d1c8fd7622e8e 100644 --- a/fixtures/gitlab.py +++ b/fixtures/gitlab.py @@ -426,6 +426,156 @@ def create_gitlab_repo( } """ +ISSUE_CLOSED_EVENT = b"""{ + "object_kind": "issue", + "event_type": "issue", + "user": { + "id": 1, + "name": "Administrator", + "username": "root", + "avatar_url": "http://www.gravatar.com/avatar/avatar.jpg", + "email": "admin@example.com" + }, + "project": { + "id": 15, + "name": "Sentry", + "description": "", + "web_url": "http://example.com/cool-group/sentry", + "avatar_url": null, + "git_ssh_url": "git@example.com:cool-group/sentry.git", + "git_http_url": "http://example.com/cool-group/sentry.git", + "namespace": "cool-group", + "visibility_level": 0, + "path_with_namespace": "cool-group/sentry", + "default_branch": "master", + "homepage": "http://example.com/cool-group/sentry", + "url": "git@example.com:cool-group/sentry.git", + "ssh_url": "git@example.com:cool-group/sentry.git", + "http_url": "http://example.com/cool-group/sentry.git" + }, + "object_attributes": { + "id": 301, + "title": "Test issue", + "assignee_ids": [], + "assignee_id": null, + "author_id": 1, + "project_id": 15, + "created_at": "2023-01-01 00:00:00 UTC", + "updated_at": "2023-01-01 00:00:00 UTC", + "position": 0, + "branch_name": null, + "description": "Test issue description", + "milestone_id": null, + "state": "closed", + "iid": 23, + "url": "http://example.com/cool-group/sentry/issues/23", + "action": "close" + }, + "assignees": [], + "labels": [] +} +""" + +ISSUE_REOPENED_EVENT = b"""{ + "object_kind": "issue", + "event_type": "issue", + "user": { + "id": 1, + "name": "Administrator", + "username": "root", + "avatar_url": "http://www.gravatar.com/avatar/avatar.jpg", + "email": "admin@example.com" + }, + "project": { + "id": 15, + "name": "Sentry", + "description": "", + "web_url": "http://example.com/cool-group/sentry", + "avatar_url": null, + "git_ssh_url": "git@example.com:cool-group/sentry.git", + "git_http_url": "http://example.com/cool-group/sentry.git", + "namespace": "cool-group", + "visibility_level": 0, + "path_with_namespace": "cool-group/sentry", + "default_branch": "master", + "homepage": "http://example.com/cool-group/sentry", + "url": "git@example.com:cool-group/sentry.git", + "ssh_url": "git@example.com:cool-group/sentry.git", + "http_url": "http://example.com/cool-group/sentry.git" + }, + "object_attributes": { + "id": 301, + "title": "Test issue", + "assignee_ids": [], + "assignee_id": null, + "author_id": 1, + "project_id": 15, + "created_at": "2023-01-01 00:00:00 UTC", + "updated_at": "2023-01-01 00:00:00 UTC", + "position": 0, + "branch_name": null, + "description": "Test issue description", + "milestone_id": null, + "state": "opened", + "iid": 23, + "url": "http://example.com/cool-group/sentry/issues/23", + "action": "reopen" + }, + "assignees": [], + "labels": [] +} +""" + +ISSUE_OPENED_EVENT = b"""{ + "object_kind": "issue", + "event_type": "issue", + "user": { + "id": 1, + "name": "Administrator", + "username": "root", + "avatar_url": "http://www.gravatar.com/avatar/avatar.jpg", + "email": "admin@example.com" + }, + "project": { + "id": 15, + "name": "Sentry", + "description": "", + "web_url": "http://example.com/cool-group/sentry", + "avatar_url": null, + "git_ssh_url": "git@example.com:cool-group/sentry.git", + "git_http_url": "http://example.com/cool-group/sentry.git", + "namespace": "cool-group", + "visibility_level": 0, + "path_with_namespace": "cool-group/sentry", + "default_branch": "master", + "homepage": "http://example.com/cool-group/sentry", + "url": "git@example.com:cool-group/sentry.git", + "ssh_url": "git@example.com:cool-group/sentry.git", + "http_url": "http://example.com/cool-group/sentry.git" + }, + "object_attributes": { + "id": 301, + "title": "Test issue", + "assignee_ids": [], + "assignee_id": null, + "author_id": 1, + "project_id": 15, + "created_at": "2023-01-01 00:00:00 UTC", + "updated_at": "2023-01-01 00:00:00 UTC", + "position": 0, + "branch_name": null, + "description": "Test issue description", + "milestone_id": null, + "state": "opened", + "iid": 23, + "url": "http://example.com/cool-group/sentry/issues/23", + "action": "open" + }, + "assignees": [], + "labels": [] +} +""" + COMPARE_RESPONSE = r""" { "commit": { diff --git a/pyproject.toml b/pyproject.toml index 60264b09ad04b8..4d323c49836886 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -82,7 +82,7 @@ dependencies = [ "rfc3339-validator>=0.1.2", "rfc3986-validator>=0.1.1", # [end] jsonschema format validators - "sentry-arroyo>=2.38.1", + "sentry-arroyo>=2.38.5", "sentry-conventions>=0.3.0", "sentry-forked-email-reply-parser>=0.5.12.post1", "sentry-kafka-schemas>=2.1.27", @@ -172,7 +172,7 @@ dev = [ "responses>=0.23.1", "ruff>=0.14.0", "selenium>=4.16.0", - "sentry-cli>=2.16.0", + "sentry-cli>=3.3.0", "sentry-covdefaults-disable-branch-coverage>=1.0.2", "sentry-devenv>=1.28.0", "django-stubs>=5.2.9", @@ -300,6 +300,10 @@ filterwarnings = [ # a deprecated constant. But, still report it as a warning so that we're # informed and can evaluate. "default:ATTRIBUTE_NAMES\\..* is deprecated.*:DeprecationWarning", + + # PyJWT 2.12.0 introduced InsecureKeyLengthWarning for HMAC keys shorter than + # 32 bytes. Tests use short secrets intentionally; production secrets are long enough. + "ignore:.*HMAC key.*below the minimum recommended length.*:jwt.warnings.InsecureKeyLengthWarning", ] looponfailroots = ["src", "tests"] diff --git a/src/sentry/api/serializers/models/rule.py b/src/sentry/api/serializers/models/rule.py index 3f62473aaed3a7..1a518cdfc17274 100644 --- a/src/sentry/api/serializers/models/rule.py +++ b/src/sentry/api/serializers/models/rule.py @@ -31,6 +31,7 @@ WorkflowDataConditionGroup, ) from sentry.workflow_engine.models.data_condition import is_slow_condition +from sentry.workflow_engine.models.data_condition_group_action import DataConditionGroupAction from sentry.workflow_engine.models.detector_workflow import DetectorWorkflow from sentry.workflow_engine.processors.workflow_fire_history import get_last_fired_dates from sentry.workflow_engine.registry import condition_handler_registry @@ -398,15 +399,13 @@ def _fetch_workflow_users(self, item_list: Sequence[Workflow]) -> dict[int, RpcU ) } - def _fetch_workflow_projects( - self, item_list: Sequence[Workflow] - ) -> dict[Workflow, set[Project]]: - workflow_to_projects: dict[Workflow, set[Project]] = defaultdict(set) + def _fetch_workflow_projects(self, item_list: Sequence[Workflow]) -> dict[int, set[Project]]: + workflow_to_projects: dict[int, set[Project]] = defaultdict(set) detector_workflows = DetectorWorkflow.objects.filter( workflow_id__in=[item.id for item in item_list] - ).prefetch_related("detector__project") - for detector_workflow in detector_workflows: - workflow_to_projects[detector_workflow.workflow].add(detector_workflow.detector.project) + ).select_related("detector__project") + for dw in detector_workflows: + workflow_to_projects[dw.workflow_id].add(dw.detector.project) return workflow_to_projects @@ -456,8 +455,23 @@ def _fetch_workflow_owner(self, workflow: Workflow) -> str | None: return actor.identifier return None - def _fetch_actions(self, condition_group: DataConditionGroup) -> BaseQuerySet[Action]: - return Action.objects.filter(dataconditiongroupaction__condition_group=condition_group) + def _fetch_actions_by_dcg(self, condition_group_ids: Sequence[int]) -> dict[int, list[Action]]: + dcg_actions = ( + DataConditionGroupAction.objects.filter( + condition_group_id__in=condition_group_ids, + ) + .exclude( + action__status__in=[ + ObjectStatus.DELETION_IN_PROGRESS, + ObjectStatus.PENDING_DELETION, + ], + ) + .select_related("action") + ) + result: dict[int, list[Action]] = defaultdict(list) + for dcg_action in dcg_actions: + result[dcg_action.condition_group_id].append(dcg_action.action) + return result def _generate_rule_conditions_filters( self, workflow: Workflow, project: Project, workflow_dcg: WorkflowDataConditionGroup @@ -570,6 +584,15 @@ def get_attrs(self, item_list: Sequence[Workflow], user, **kwargs): # Bulk fetch workflow -> rule ids workflow_rule_ids = self._fetch_workflow_rule_ids(item_list) + # Bulk fetch actions for all condition groups across all workflows + all_dcg_ids: list[int] = [] + for wf in workflows: + all_dcg_ids.extend( + wdcg.condition_group_id + for wdcg in wf.prefetched_wdcgs # type: ignore[attr-defined] + ) + actions_by_dcg = self._fetch_actions_by_dcg(all_dcg_ids) + last_triggered_lookup: dict[int, datetime] = {} if "lastTriggered" in self.expand: last_triggered_lookup = self._fetch_workflow_last_triggered(item_list) @@ -583,7 +606,7 @@ def get_attrs(self, item_list: Sequence[Workflow], user, **kwargs): result[workflow]["owner"] = owner result[workflow]["environment"] = workflow.environment - result[workflow]["projects"] = list(workflow_to_projects[workflow]) + result[workflow]["projects"] = list(workflow_to_projects[workflow.id]) result[workflow]["rule_id"] = workflow_rule_ids.get( workflow.id, get_fake_id_from_object_id(workflow.id) ) @@ -608,7 +631,7 @@ def get_attrs(self, item_list: Sequence[Workflow], user, **kwargs): result[workflow]["filter_match"] = workflow_dcg.condition_group.logic_type # build up actions data - actions = self._fetch_actions(workflow_dcg.condition_group) + actions = actions_by_dcg.get(workflow_dcg.condition_group_id, []) action_to_handler = {} for action in actions: try: diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index 02e2e9f64e8827..cbd1bd453e7908 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -3016,7 +3016,11 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]: SENTRY_PREPROD_ARTIFACT_EVENTS_FUTURES_MAX_LIMIT = 10000 # How long we should wait for a gateway proxy request to return before giving up -GATEWAY_PROXY_TIMEOUT: int | None = None +GATEWAY_PROXY_TIMEOUT: int | None = ( + int(os.environ["SENTRY_APIGW_PROXY_TIMEOUT"]) + if os.environ.get("SENTRY_APIGW_PROXY_TIMEOUT") + else None +) SENTRY_SLICING_LOGICAL_PARTITION_COUNT = 256 # This maps a Sliceable for slicing by name and (lower logical partition, upper physical partition) diff --git a/src/sentry/deletions/defaults/organization.py b/src/sentry/deletions/defaults/organization.py index 879f46cbcf6b89..96c82b1103e8eb 100644 --- a/src/sentry/deletions/defaults/organization.py +++ b/src/sentry/deletions/defaults/organization.py @@ -47,6 +47,7 @@ def get_child_relations(self, instance: Organization) -> list[BaseRelation]: AlertRule, Release, Project, + Workflow, Environment, Dashboard, TeamKeyTransaction, @@ -64,7 +65,6 @@ def get_child_relations(self, instance: Organization) -> list[BaseRelation]: task=DiscoverSavedQueryDeletionTask, ) ) - relations.append(ModelRelation(Workflow, {"organization_id": instance.id})) return relations diff --git a/src/sentry/event_manager.py b/src/sentry/event_manager.py index d366f9f37ff54d..5f7a3bbf52a87a 100644 --- a/src/sentry/event_manager.py +++ b/src/sentry/event_manager.py @@ -1447,7 +1447,7 @@ def handle_existing_grouphash( # this function had races around group creation which made this race # more user visible. For more context, see 84c6f75a and d0e22787, as # well as GH-5085. - group = Group.objects.get(id=existing_grouphash.group_id) + group = Group.objects.get_from_cache(id=existing_grouphash.group_id, use_replica=False) # As far as we know this has never happened, but in theory at least, the error event hashing # algorithm and other event hashing algorithms could come up with the same hash value in the diff --git a/src/sentry/features/temporary.py b/src/sentry/features/temporary.py index 7d43ad0f34b8bc..0bf28bcf3fc8de 100644 --- a/src/sentry/features/temporary.py +++ b/src/sentry/features/temporary.py @@ -356,6 +356,8 @@ def register_temporary_features(manager: FeatureManager) -> None: manager.add("organizations:insights-modules-use-eap", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) # Enable access to insights metrics alerts manager.add("organizations:insights-alerts", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) + # Enable data browsing widget unfurl + manager.add("organizations:data-browsing-widget-unfurl", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) # Enable dual-write of Seer project preferences to Sentry DB and Seer API manager.add("organizations:seer-project-settings-dual-write", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) # Enable public RPC endpoint for local seer development @@ -478,8 +480,6 @@ def register_temporary_features(manager: FeatureManager) -> None: manager.add("organizations:cache-detectors-by-data-source", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) # Enable single trace summary manager.add("organizations:single-trace-summary", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) - # Enable seeing upsell modal when clicking upgrade for multi-org - manager.add("organizations:github-multi-org-upsell-modal", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) # Enables CMD+K supercharged (omni search) manager.add("organizations:cmd-k-supercharged", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) # Enables DSN lookup in CMD+K palette @@ -492,6 +492,8 @@ def register_temporary_features(manager: FeatureManager) -> None: manager.add("organizations:conduit-demo", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) # Enable hard timeout alarm for webhooks manager.add("organizations:sentry-app-webhook-hard-timeout", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) + # Enable circuit breaker for webhook endpoint failure detection + manager.add("organizations:sentry-app-webhook-circuit-breaker", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) # Enables organization access to the new notification platform manager.add("organizations:notification-platform.internal-testing", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) diff --git a/src/sentry/grouping/ingest/seer.py b/src/sentry/grouping/ingest/seer.py index 3fdcc2420ee2fb..ec9a4147c06acd 100644 --- a/src/sentry/grouping/ingest/seer.py +++ b/src/sentry/grouping/ingest/seer.py @@ -316,7 +316,6 @@ def _build_seer_request( "exception_type": filter_null_from_string(exception_type) if exception_type else None, "k": options.get("seer.similarity.ingest.num_matches_to_request"), "referrer": "ingest", - "use_reranking": options.get("seer.similarity.ingest.use_reranking"), "model": model_version, "training_mode": training_mode, "platform": event.platform or "unknown", @@ -413,8 +412,6 @@ def get_seer_similar_issues( # By asking Seer to find zero matches, we can trick it into thinking there aren't # any, thereby forcing it to create the record "k": 0, - # Turn off re-ranking to speed up the process of finding nothing - "use_reranking": False, } # TODO: Temporary log to prove things are working as they should. This should come in a pair diff --git a/src/sentry/grouping/parameterization.py b/src/sentry/grouping/parameterization.py index e0a1031b0adfb9..dca27064f4e041 100644 --- a/src/sentry/grouping/parameterization.py +++ b/src/sentry/grouping/parameterization.py @@ -1,7 +1,7 @@ import dataclasses import re from collections import defaultdict -from collections.abc import Iterable, Sequence +from collections.abc import Sequence from sentry.utils import metrics @@ -238,7 +238,23 @@ def _get_pattern(self, raw_pattern: str) -> str: """, ), ParameterizationRegex(name="float", raw_pattern=r"""-\d+\.\d+\b | \b\d+\.\d+\b"""), - ParameterizationRegex(name="int", raw_pattern=r"""-\d+\b | \b\d+\b"""), + ParameterizationRegex( + name="int", + raw_pattern=r""" + ( + # Regular word boundary for positive ints + \b + | + # Alphanumeric negative lookbehind for negative ints to ensure a dash is only + # considered a minus sign if it doesn't connect two alphanumeric strings. (No word + # boundary here because the dash serves as the word boundary, since it's not a word + # character.) + (? str: ] -# Patterns to use for each match type when not in experimental mode. -DEFAULT_PARAMETERIZATION_REGEXES_MAP = {r.name: r.pattern for r in DEFAULT_PARAMETERIZATION_REGEXES} - -# Patterns to use when in experimental mode. If no experimental pattern exists for a given type of -# match, falls back to the default pattern. -EXPERIMENTAL_PARAMETERIZATION_REGEXES_MAP = { - r.name: r.experimental_pattern if r.experimental_pattern else r.pattern - for r in DEFAULT_PARAMETERIZATION_REGEXES -} - - class Parameterizer: def __init__( self, + # List of `ParameterizationRegex` objects defining the regexes to use. If nothing is passed, + # the default set will be used. + regexes: Sequence[ParameterizationRegex] = DEFAULT_PARAMETERIZATION_REGEXES, # List of `ParameterizationRegex.name` values, used to selectively enable pattern types. To # use all available parameterization, omit this argument. - regex_pattern_keys: Sequence[str] | None = None, + regex_keys: Sequence[str] | None = None, # Whether to use experimental patterns, if available. (Pattern types without an experimental # pattern will fall back to the standard pattern.) use_experimental_regexes: bool = False, ): - self._experimental = ( + # Filter regexes by the specified keys, if given + if regex_keys: + regexes = [r for r in regexes if r.name in regex_keys] + + self.is_experimental = ( use_experimental_regexes # Only mark the parameterizer as experimental if there are actually any experiments # running. If there aren't, then both parameterizers use the default regex patterns, so # the "experimental" parameterizer isn't actually experimental. - and EXPERIMENTAL_PARAMETERIZATION_REGEXES_MAP != DEFAULT_PARAMETERIZATION_REGEXES_MAP - ) - self._parameterization_regex = self._make_regex_from_patterns( - regex_pattern_keys or DEFAULT_PARAMETERIZATION_REGEXES_MAP.keys() + and any(r.experimental_pattern is not None for r in regexes) ) - def _make_regex_from_patterns(self, pattern_keys: Iterable[str]) -> re.Pattern[str]: - """ - Takes list of pattern keys and returns a compiled regex pattern that matches any of them. - - @param pattern_keys: A list of keys to match in the _parameterization_regex_components dict. - @returns: A compiled regex pattern that matches any of the given keys. - @raises: KeyError on pattern key not in the _parameterization_regex_components dict - - The `(?x)` tells the regex compiler to ignore comments and unescaped whitespace, - so we can use newlines and indentation for better legibility in patterns above. - """ + if self.is_experimental: + pattern_strings = [ + r.experimental_pattern if r.experimental_pattern else r.pattern for r in regexes + ] + else: + pattern_strings = [r.pattern for r in regexes] - regexes_map = ( - EXPERIMENTAL_PARAMETERIZATION_REGEXES_MAP - if self._experimental - else DEFAULT_PARAMETERIZATION_REGEXES_MAP + # Combine the individual patterns into one giant regex to check against. (This is faster + # than checking each pattern individually because it entails less overhead.) + self.combined_regex = re.compile( + # The `(?x)` tells the regex compiler to ignore comments and unescaped whitespace, so we + # can use newlines and indentation for better legibility when defining regexes + rf"(?x){'|'.join(pattern_strings)}" ) - return re.compile(rf"(?x){'|'.join(regexes_map[k] for k in pattern_keys)}") - def parameterize(self, input_str: str) -> str: """ Replace all regex matches in the input string with placeholder strings, using the regexes @@ -326,24 +330,32 @@ def parameterize(self, input_str: str) -> str: matches_counter: defaultdict[str, int] = defaultdict(int) def _handle_regex_match(match: re.Match[str]) -> str: - # Find the first (should be only) non-None match entry, and sub in the placeholder. For - # example, given the groupdict item `('hex', '0x40000015')`, this returns '' as a - # replacement for the original value in the string. - for key, value in match.groupdict().items(): - if value is not None: - matches_counter[key] += 1 - return f"<{key}>" - return "" + # Since + # a) our regex consists of a bunch of named capturing groups separated by `|`, + # b) no other capturing groups in the regex are named, and + # c) there's nothing else in the regex, + # there should be exactly one named matching group, making the last matching group also + # the only matching group. + matched_key = match.lastgroup + orig_value = match.groupdict().get( + matched_key or "" # Empty string for mypy appeasment + ) + + if not matched_key or not orig_value: # Insurance - shouldn't happen IRL + return "" + + matches_counter[matched_key] += 1 + return f"<{matched_key}>" with metrics.timer( - "grouping.parameterize", tags={"experimental": self._experimental} + "grouping.parameterize", tags={"experimental": self.is_experimental} ) as metric_tags: - parameterized = self._parameterization_regex.sub(_handle_regex_match, input_str) + parameterized = self.combined_regex.sub(_handle_regex_match, input_str) metric_tags["changed"] = parameterized != input_str - for key, value in matches_counter.items(): + for regex_key, count in matches_counter.items(): # Track the kinds of replacements being made - metrics.incr("grouping.value_parameterized", amount=value, tags={"key": key}) + metrics.incr("grouping.value_parameterized", amount=count, tags={"key": regex_key}) return parameterized diff --git a/src/sentry/hybridcloud/apigateway_async/proxy.py b/src/sentry/hybridcloud/apigateway_async/proxy.py index 7fed6928dfaed8..254ab6bf59dfac 100644 --- a/src/sentry/hybridcloud/apigateway_async/proxy.py +++ b/src/sentry/hybridcloud/apigateway_async/proxy.py @@ -41,7 +41,7 @@ logger = logging.getLogger(__name__) -proxy_client = httpx.AsyncClient() +proxy_client = httpx.AsyncClient(timeout=httpx.Timeout(5.0, read=60.0)) circuitbreakers = CircuitBreakerManager() # Endpoints that handle uploaded files have higher timeouts configured @@ -189,7 +189,7 @@ async def proxy_cell_request( headers=header_dict, params=dict(query_params) if query_params is not None else None, content=_stream_request(data) if data else None, # type: ignore[arg-type] - timeout=timeout, + timeout=timeout or httpx.USE_CLIENT_DEFAULT, ) resp = await proxy_client.send(req, stream=True, follow_redirects=False) if resp.status_code >= 502: diff --git a/src/sentry/integrations/github/integration.py b/src/sentry/integrations/github/integration.py index 04f0d4693bf30e..86cdfaf189359f 100644 --- a/src/sentry/integrations/github/integration.py +++ b/src/sentry/integrations/github/integration.py @@ -3,19 +3,21 @@ import logging import re from collections.abc import Callable, Mapping, MutableMapping, Sequence +from dataclasses import dataclass from enum import StrEnum -from typing import Any, TypedDict +from typing import Any, NotRequired, TypedDict from urllib.parse import parse_qsl from django.db.models import Count -from django.http import HttpResponse from django.http.request import HttpRequest from django.http.response import HttpResponseBase, HttpResponseRedirect from django.urls import reverse from django.utils.text import slugify from django.utils.translation import gettext_lazy as _ +from rest_framework.serializers import CharField from sentry import features, options +from sentry.api.serializers.rest_framework.base import CamelSnakeSerializer from sentry.constants import ObjectStatus from sentry.http import safe_urlopen, safe_urlread from sentry.identity.github.provider import GitHubIdentityProvider @@ -62,7 +64,12 @@ RpcOrganization, RpcUserOrganizationContext, ) -from sentry.pipeline.views.base import PipelineView, render_react_view +from sentry.pipeline.types import PipelineStepResult +from sentry.pipeline.views.base import ( + ApiPipelineSteps, + PipelineView, + render_react_view, +) from sentry.shared_integrations.constants import ERR_INTERNAL, ERR_UNAUTHORIZED from sentry.shared_integrations.exceptions import ApiError, ApiInvalidRequestError, IntegrationError from sentry.snuba.referrer import Referrer @@ -177,6 +184,7 @@ class GithubInstallationInfo(TypedDict): installation_id: str github_account: str avatar_url: str + count: NotRequired[int] def build_repository_query(metadata: Mapping[str, Any], name: str, query: str) -> bytes: @@ -789,6 +797,12 @@ def get_pipeline_views( ]: return [OAuthLoginView(), GithubOrganizationSelection(), GitHubInstallation()] + def get_pipeline_api_steps(self) -> ApiPipelineSteps[IntegrationPipeline]: + return [ + OAuthLoginApiStep(), + GithubOrganizationSelectionApiStep(), + ] + def get_installation_info(self, installation_id: str) -> Mapping[str, Any]: resp: Mapping[str, Any] = self.client.get_installation_info(installation_id=installation_id) return resp @@ -844,133 +858,491 @@ class GitHubInstallationError(StrEnum): USER_MISMATCH = "Authenticated user is not the same as who installed the app." MISSING_INTEGRATION = "Integration does not exist." INVALID_INSTALLATION = "User does not have access to given installation." + MISSING_OWNER_PRIVILEGES = ( + "Your GitHub account does not have owner privileges for the chosen organization." + ) FEATURE_NOT_AVAILABLE = "Your organization does not have access to this feature." MISSING_ORGANIZATION = "You must be logged into an organization to access this feature." +def get_install_app_url() -> str: + name = options.get("github-app.name") + return f"https://github.com/apps/{slugify(name)}" + + def record_event(event: IntegrationPipelineViewType): return IntegrationPipelineViewEvent( event, IntegrationDomain.SOURCE_CODE_MANAGEMENT, GitHubIntegrationProvider.key ) -class OAuthLoginView: - client: GithubSetupApiClient +class GitHubPipelineError(Exception): + """Raised when a GitHub pipeline step fails validation.""" + + def __init__(self, message: GitHubInstallationError) -> None: + self.message = message + super().__init__(str(message)) + + +@dataclass +class GitHubOAuthLoginResult: + authenticated_user: str + installation_info: list[GithubInstallationInfo] + + +def _get_owner_github_organizations(client: GithubSetupApiClient) -> list[str]: + user_org_membership_details = client.get_organization_memberships_for_user() + return [ + gh_org.get("organization", {}).get("login") + for gh_org in user_org_membership_details + if ( + gh_org.get("role", "").lower() == "admin" + and gh_org.get("state", "").lower() == "active" + ) + ] + + +def _get_eligible_multi_org_installations( + client: GithubSetupApiClient, owner_orgs: list[str] +) -> list[GithubInstallationInfo]: + installed_orgs = client.get_user_info_installations() + return [ + { + "installation_id": str(installation.get("id")), + "github_account": installation.get("account").get("login"), + "avatar_url": installation.get("account").get("avatar_url"), + } + for installation in installed_orgs["installations"] + if ( + installation.get("account").get("login") in owner_orgs + or installation.get("target_type") == "User" + ) + ] + + +def _build_github_oauth_url(pipeline: IntegrationPipeline) -> str: + ghip = GitHubIdentityProvider() + redirect_uri = absolute_uri( + reverse( + "sentry-extension-setup", + kwargs={"provider_id": IntegrationProviderSlug.GITHUB.value}, + ) + ) + return ( + f"{ghip.get_oauth_authorize_url()}" + f"?client_id={ghip.get_oauth_client_id()}" + f"&state={pipeline.signature}" + f"&redirect_uri={redirect_uri}" + ) + + +def exchange_github_oauth( + code: str, + fetch_installations: bool = True, +) -> GitHubOAuthLoginResult: + """ + Exchange an OAuth code for a GitHub access token, fetch the authenticated + user's info, and optionally determine eligible installations. + + Raises GitHubPipelineError on failure. + """ + ghip = GitHubIdentityProvider() + data = { + "code": code, + "client_id": ghip.get_oauth_client_id(), + "client_secret": ghip.get_oauth_client_secret(), + } + + req = safe_urlopen(url=ghip.get_oauth_access_token_url(), data=data) + try: + body = safe_urlread(req).decode("utf-8") + payload = dict(parse_qsl(body)) + except Exception: + payload = {} + + if "access_token" not in payload: + raise GitHubPipelineError(GitHubInstallationError.MISSING_TOKEN) + + client = GithubSetupApiClient(access_token=payload["access_token"]) + authenticated_user_info = client.get_user_info() + + installation_info: list[GithubInstallationInfo] = [] + if fetch_installations: + owner_orgs = _get_owner_github_organizations(client) + installation_info = _get_eligible_multi_org_installations(client, owner_orgs) + + if "login" not in authenticated_user_info: + raise GitHubPipelineError(GitHubInstallationError.MISSING_LOGIN) + + return GitHubOAuthLoginResult( + authenticated_user=authenticated_user_info["login"], + installation_info=installation_info, + ) + + +def _build_installation_info_with_counts( + installation_info: list[GithubInstallationInfo], +) -> list[GithubInstallationInfo]: + """ + Enriches an installation info list with Sentry org install counts and appends + the 'install on new GitHub org' sentinel option. + """ + installation_ids = [i["installation_id"] for i in installation_info] + counts_qs = ( + OrganizationIntegration.objects.filter( + integration__provider=GitHubIntegrationProvider.key, + integration__external_id__in=installation_ids, + ) + .values("integration__external_id") + .annotate(count=Count("id")) + ) + counts_dict = {item["integration__external_id"]: item["count"] for item in counts_qs} + + result: list[GithubInstallationInfo] = [] + for installation in installation_info: + result.append( + { + **installation, + "count": counts_dict.get(installation["installation_id"], 0), + } + ) + result.append( + { + "installation_id": "-1", + "github_account": "Integrate with a new GitHub organization", + "avatar_url": "", + "count": 0, + } + ) + return result + + +def validate_org_installation_choice( + chosen_installation_id: str, + pipeline: IntegrationPipeline, +) -> None: + """ + Validates a chosen GitHub installation can be linked to this Sentry org. + Checks multi-org feature flag, install count, org consistency, and that the + installation belongs to the user. Binds chosen_installation to pipeline state. + + Raises GitHubPipelineError on validation failure. + """ + installation_info: list[GithubInstallationInfo] = ( + pipeline.fetch_state("existing_installation_info") or [] + ) + + has_scm_multi_org = features.has( + "organizations:integrations-scm-multi-org", + organization=pipeline.organization, + ) + install_count = OrganizationIntegration.objects.filter( + integration__provider=GitHubIntegrationProvider.key, + integration__external_id=chosen_installation_id, + ).count() + if not ((install_count == 0) or has_scm_multi_org): + raise GitHubPipelineError(GitHubInstallationError.FEATURE_NOT_AVAILABLE) + + installing_org_slug = pipeline.fetch_state("installing_organization_slug") + if not (installing_org_slug is not None and installing_org_slug == pipeline.organization.slug): + raise GitHubPipelineError(GitHubInstallationError.FEATURE_NOT_AVAILABLE) + + valid_ids = [i["installation_id"] for i in installation_info] + if chosen_installation_id not in valid_ids: + raise GitHubPipelineError(GitHubInstallationError.MISSING_OWNER_PRIVILEGES) + + pipeline.bind_state("chosen_installation", chosen_installation_id) + + +def validate_github_installation( + pipeline: IntegrationPipeline, +) -> str | None: + """ + Resolves the installation_id from pipeline state (handling chosen_installation), + checks for pending deletion and user mismatch against existing integrations. + + Returns the resolved installation_id, or None if the user needs to install the + GitHub App first (caller should redirect). + + Raises GitHubPipelineError on validation failure. + """ + chosen_installation_id = pipeline.fetch_state("chosen_installation") + if chosen_installation_id is not None: + pipeline.bind_state("installation_id", chosen_installation_id) + + installation_id = pipeline.fetch_state("installation_id") + if installation_id is None: + return None + + pending_deletion = OrganizationIntegration.objects.filter( + integration__provider=GitHubIntegrationProvider.key, + organization_id=pipeline.organization.id, + status=ObjectStatus.PENDING_DELETION, + ).exists() + if pending_deletion: + raise GitHubPipelineError(GitHubInstallationError.PENDING_DELETION) + + try: + integration = Integration.objects.get( + external_id=installation_id, status=ObjectStatus.ACTIVE + ) + except Integration.DoesNotExist: + # The installation.created webhook from GitHub normally creates the + # Integration record (with sender metadata) before the user reaches + # this point. If it doesn't exist yet, the webhook likely hasn't + # been processed. We proceed without the sender validation that the + # existing-integration path provides below. In theory an attacker + # could race the webhook to link an installation they don't own, + # but the window is extremely narrow (they'd need to predict the + # installation_id before the webhook arrives). + return installation_id + + if ( + chosen_installation_id is None + and pipeline.fetch_state("github_authenticated_user") + != integration.metadata["sender"]["login"] + ): + raise GitHubPipelineError(GitHubInstallationError.USER_MISMATCH) + + return installation_id + + +class OAuthLoginStepData(TypedDict): + oauthUrl: str + + +class OAuthLoginValidatedData(TypedDict): + code: str + state: str + installation_id: NotRequired[str] + + +class OAuthLoginSerializer(CamelSnakeSerializer): + code = CharField(required=True) + state = CharField(required=True) + # GitHub includes installation_id in the OAuth callback URL when the user + # installs the app from GitHub's side rather than from Sentry. + installation_id = CharField(required=False) + + +class OAuthLoginApiStep: + """ + Initiates and completes the GitHub OAuth authorization flow. + + On GET (via get_step_data): returns the GitHub OAuth authorize URL. The + frontend should open this in a popup/redirect so the user can authorize. + + On POST (via handle_post): receives the OAuth callback params (code, state, + and optionally installation_id), exchanges the code for an access token, + verifies the authenticated GitHub user, and discovers existing GitHub App + installations the user has access to. + + If installation_id is present in the callback, it is bound to pipeline + state early. This handles the case where the user started from GitHub's + side (installing the app directly on GitHub), which redirects back with + installation_id before OAuth has happened. The installation_id persists + in pipeline state across the OAuth redirect, so the later installation + validation step can find it without requiring user action. + """ + + step_name = "oauth_login" + + def get_step_data( + self, pipeline: IntegrationPipeline, request: HttpRequest + ) -> OAuthLoginStepData: + return {"oauthUrl": _build_github_oauth_url(pipeline)} + + def get_serializer_cls(self) -> type: + return OAuthLoginSerializer + + def handle_post( + self, + validated_data: OAuthLoginValidatedData, + pipeline: IntegrationPipeline, + request: HttpRequest, + ) -> PipelineStepResult: + with record_event(IntegrationPipelineViewType.OAUTH_LOGIN).capture() as lifecycle: + lifecycle.add_extra("organization_id", pipeline.organization.id) + + if installation_id := validated_data.get("installation_id"): + pipeline.bind_state("installation_id", installation_id) + + if validated_data["state"] != pipeline.signature: + lifecycle.record_failure(GitHubInstallationError.INVALID_STATE) + return PipelineStepResult.error(GitHubInstallationError.INVALID_STATE) + + try: + result = exchange_github_oauth(code=validated_data["code"]) + except GitHubPipelineError as e: + lifecycle.record_failure(e.message) + return PipelineStepResult.error(e.message) + + if result.installation_info: + pipeline.bind_state("existing_installation_info", result.installation_info) + pipeline.bind_state("github_authenticated_user", result.authenticated_user) + return PipelineStepResult.advance() + + +class GithubInstallationApiInfo(TypedDict): + installationId: str + githubAccount: str + avatarUrl: str + count: NotRequired[int | None] + + +class OrgSelectionStepData(TypedDict): + installAppUrl: str + installationInfo: list[GithubInstallationApiInfo] + + +class OrgSelectionValidatedData(TypedDict): + chosen_installation_id: NotRequired[str] + installation_id: NotRequired[str] + + +class OrgSelectionSerializer(CamelSnakeSerializer): + chosen_installation_id = CharField(required=False, allow_blank=True) + installation_id = CharField(required=False) + + +class GithubOrganizationSelectionApiStep: + """ + Connects a GitHub App installation to the user's Sentry org. + + On GET: returns the list of GitHub orgs/accounts where the authenticated + user has the app installed, enriched with how many Sentry orgs already use + each installation. + + On POST: handles three cases: + 1. User selected an existing installation (chosen_installation_id) — + validates multi-org eligibility, org consistency, and ownership. + 2. User completed the GitHub App install popup and the frontend posted + back the new installation_id. + 3. The installation_id is already in pipeline state from the user + clicking "Install" on github.com and being redirected into this flow. + + Cases 2 and 3 both go through validate_github_installation. If no + installation_id is available yet, returns a stay result prompting the + user to install the app. + """ + + step_name = "org_selection" + + def get_step_data( + self, pipeline: IntegrationPipeline, request: HttpRequest + ) -> OrgSelectionStepData: + install_app_url = get_install_app_url() + + installation_info: list[GithubInstallationInfo] = list( + pipeline.fetch_state("existing_installation_info") or [] + ) + if not installation_info: + return {"installationInfo": [], "installAppUrl": install_app_url} + + # Bind the installing org slug so we can validate it hasn't changed when + # the user submits their choice (same logic as the template flow). + pipeline.bind_state("installing_organization_slug", pipeline.organization.slug) + + enriched = _build_installation_info_with_counts(installation_info) + return { + "installAppUrl": install_app_url, + "installationInfo": [ + { + "installationId": info["installation_id"], + "githubAccount": info["github_account"], + "avatarUrl": info.get("avatar_url", ""), + "count": info.get("count"), + } + for info in enriched + ], + } + + def get_serializer_cls(self) -> type | None: + return OrgSelectionSerializer + + def handle_post( + self, + validated_data: OrgSelectionValidatedData, + pipeline: IntegrationPipeline, + request: HttpRequest, + ) -> PipelineStepResult: + with record_event( + IntegrationPipelineViewType.ORGANIZATION_SELECTION + ).capture() as lifecycle: + lifecycle.add_extra("organization_id", pipeline.organization.id) + + # If the user completed the GitHub App install popup, bind the + # installation_id and validate it + if post_installation_id := validated_data.get("installation_id"): + pipeline.bind_state("installation_id", post_installation_id) + # If the user selected a GitHub app installation from the list + # validate their choice and bind the installation_id. + chosen_installation_id = validated_data.get("chosen_installation_id") + if chosen_installation_id: + try: + validate_org_installation_choice(chosen_installation_id, pipeline) + except GitHubPipelineError as e: + lifecycle.record_failure(e.message) + return PipelineStepResult.error(e.message) + pipeline.bind_state("installation_id", chosen_installation_id) + + # Validate whatever installation_id is in pipeline state — handles + # pending deletion check and user mismatch against existing integrations. + try: + installation_id = validate_github_installation(pipeline) + except GitHubPipelineError as e: + lifecycle.record_failure(e.message) + return PipelineStepResult.error(e.message) + + if installation_id is None: + return PipelineStepResult.stay(data={"installAppUrl": get_install_app_url()}) + + return PipelineStepResult.advance() + + +class OAuthLoginView: def dispatch(self, request: HttpRequest, pipeline: IntegrationPipeline) -> HttpResponseBase: with record_event(IntegrationPipelineViewType.OAUTH_LOGIN).capture() as lifecycle: - self.active_user_organization = determine_active_organization(request) + active_user_organization = determine_active_organization(request) lifecycle.add_extra( "organization_id", - ( - self.active_user_organization.organization.id - if self.active_user_organization - else None - ), + (active_user_organization.organization.id if active_user_organization else None), ) - ghip = GitHubIdentityProvider() - github_client_id = ghip.get_oauth_client_id() - github_client_secret = ghip.get_oauth_client_secret() - installation_id = request.GET.get("installation_id") if installation_id: pipeline.bind_state("installation_id", installation_id) if not request.GET.get("state"): - state = pipeline.signature + return HttpResponseRedirect(_build_github_oauth_url(pipeline)) - redirect_uri = absolute_uri( - reverse( - "sentry-extension-setup", - kwargs={"provider_id": IntegrationProviderSlug.GITHUB.value}, - ) - ) - return HttpResponseRedirect( - f"{ghip.get_oauth_authorize_url()}?client_id={github_client_id}&state={state}&redirect_uri={redirect_uri}" - ) - - # At this point, we are past the GitHub "authorize" step if request.GET.get("state") != pipeline.signature: lifecycle.record_failure(GitHubInstallationError.INVALID_STATE) return error( request, - self.active_user_organization, + active_user_organization, error_short=GitHubInstallationError.INVALID_STATE, ) - # similar to OAuth2CallbackView.get_token_params - data = { - "code": request.GET.get("code"), - "client_id": github_client_id, - "client_secret": github_client_secret, - } - - # similar to OAuth2CallbackView.exchange_token - req = safe_urlopen(url=ghip.get_oauth_access_token_url(), data=data) try: - body = safe_urlread(req).decode("utf-8") - payload = dict(parse_qsl(body)) - except Exception: - payload = {} - - if "access_token" not in payload: - lifecycle.record_failure(GitHubInstallationError.MISSING_TOKEN) - return error( - request, - self.active_user_organization, - error_short=GitHubInstallationError.MISSING_TOKEN, - ) - self.client = GithubSetupApiClient(access_token=payload["access_token"]) - authenticated_user_info = self.client.get_user_info() - - if self.active_user_organization is not None: - owner_orgs = self._get_owner_github_organizations() - - installation_info = self._get_eligible_multi_org_installations( - owner_orgs=owner_orgs + result = exchange_github_oauth( + code=request.GET.get("code", ""), + fetch_installations=active_user_organization is not None, ) - pipeline.bind_state("existing_installation_info", installation_info) - - if "login" not in authenticated_user_info: - lifecycle.record_failure(GitHubInstallationError.MISSING_LOGIN) + except GitHubPipelineError as e: + lifecycle.record_failure(e.message) return error( request, - self.active_user_organization, - error_short=GitHubInstallationError.MISSING_LOGIN, + active_user_organization, + error_short=e.message, + error_long=e.message, ) - pipeline.bind_state("github_authenticated_user", authenticated_user_info["login"]) - return pipeline.next_step() - - def _get_owner_github_organizations(self) -> list[str]: - user_org_membership_details = self.client.get_organization_memberships_for_user() - - return [ - gh_org.get("organization", {}).get("login") - for gh_org in user_org_membership_details - if ( - gh_org.get("role", "").lower() == "admin" - and gh_org.get("state", "").lower() == "active" - ) - ] - def _get_eligible_multi_org_installations( - self, owner_orgs: list[str] - ) -> list[GithubInstallationInfo]: - installed_orgs = self.client.get_user_info_installations() - - return [ - { - "installation_id": str(installation.get("id")), - "github_account": installation.get("account").get("login"), - "avatar_url": installation.get("account").get("avatar_url"), - } - for installation in installed_orgs["installations"] - if ( - installation.get("account").get("login") in owner_orgs - or installation.get("target_type") == "User" - ) - ] + if result.installation_info: + pipeline.bind_state("existing_installation_info", result.installation_info) + pipeline.bind_state("github_authenticated_user", result.authenticated_user) + return pipeline.next_step() class GithubOrganizationSelection: @@ -997,79 +1369,23 @@ def dispatch(self, request: HttpRequest, pipeline: IntegrationPipeline) -> HttpR if len(installation_info) == 0: return pipeline.next_step() - # add information about number of org integrations per installation - installation_ids = [ - installation["installation_id"] for installation in installation_info - ] - integration_install_counts = ( - OrganizationIntegration.objects.filter( - integration__provider=GitHubIntegrationProvider.key, - integration__external_id__in=installation_ids, - ) - .values("integration__external_id") - .annotate(count=Count("id")) - ) - integration_install_counts_dict = { - item["integration__external_id"]: item["count"] - for item in integration_install_counts - } - for installation in installation_info: - installation["count"] = integration_install_counts_dict.get( - installation["installation_id"], 0 - ) - - # add an option for users to install on a new GH organization - installation_info.append( - { - "installation_id": "-1", - "github_account": "Integrate with a new GitHub organization", - "avatar_url": "", - "count": 0, - } - ) + installation_info = _build_installation_info_with_counts(installation_info) if chosen_installation_id := request.GET.get("chosen_installation_id"): if chosen_installation_id == "-1": return pipeline.next_step() - # NOTE: there may still be a race condition here where multiple orgs read the same count (0) - # the org integration creation logic is in finish_pipeline - can_install_chosen_installation = ( - integration_install_counts_dict.get(chosen_installation_id, 0) == 0 - ) or has_scm_multi_org - - # Validate the same org is installing and that they have the multi org feature - installing_organization_slug = pipeline.fetch_state("installing_organization_slug") - is_same_installing_org = ( - (installing_organization_slug is not None) - and installing_organization_slug - == self.active_user_organization.organization.slug - ) - - if not can_install_chosen_installation or not is_same_installing_org: - lifecycle.record_failure(GitHubInstallationError.FEATURE_NOT_AVAILABLE) - return error( - request, - self.active_user_organization, - error_short=GitHubInstallationError.FEATURE_NOT_AVAILABLE, - ) - - # Verify that the given GH installation belongs to the person installing the pipeline - installation_ids = [ - installation["installation_id"] for installation in installation_info - ] - if chosen_installation_id not in installation_ids: - lifecycle.record_failure( - failure_reason=GitHubInstallationError.INVALID_INSTALLATION - ) + try: + validate_org_installation_choice(chosen_installation_id, pipeline) + except GitHubPipelineError as e: + lifecycle.record_failure(e.message) return error( request, self.active_user_organization, - error_short=GitHubInstallationError.INVALID_INSTALLATION, - error_long=ERR_INTEGRATION_INVALID_INSTALLATION, + error_short=e.message, + error_long=e.message, ) - pipeline.bind_state("chosen_installation", chosen_installation_id) return pipeline.next_step() pipeline.bind_state( "installing_organization_slug", self.active_user_organization.organization.slug @@ -1093,85 +1409,39 @@ def dispatch(self, request: HttpRequest, pipeline: IntegrationPipeline) -> HttpR class GitHubInstallation: - def get_app_url(self) -> str: - name = options.get("github-app.name") - return f"https://github.com/apps/{slugify(name)}" - def dispatch(self, request: HttpRequest, pipeline: IntegrationPipeline) -> HttpResponseBase: with record_event(IntegrationPipelineViewType.GITHUB_INSTALLATION).capture() as lifecycle: - self.active_user_organization = determine_active_organization(request) - - chosen_installation_id = pipeline.fetch_state("chosen_installation") - if chosen_installation_id is not None: - pipeline.bind_state("installation_id", chosen_installation_id) - - installation_id = pipeline.fetch_state("installation_id") or request.GET.get( - "installation_id", None - ) - if installation_id is None: - return HttpResponseRedirect(self.get_app_url()) - - pipeline.bind_state("installation_id", installation_id) - + active_user_organization = determine_active_organization(request) lifecycle.add_extra( "organization_id", ( - self.active_user_organization.organization.id - if self.active_user_organization is not None + active_user_organization.organization.id + if active_user_organization is not None else None ), ) - error_page = self.check_pending_integration_deletion(request=request) - if error_page is not None: - lifecycle.record_failure(GitHubInstallationError.PENDING_DELETION) - return error_page - - if self.active_user_organization is not None: - try: - integration = Integration.objects.get( - external_id=installation_id, status=ObjectStatus.ACTIVE - ) - except Integration.DoesNotExist: - return pipeline.next_step() - - # Check that the authenticated GitHub user is the same as who installed the app. - if ( - chosen_installation_id is None - and pipeline.fetch_state("github_authenticated_user") - != integration.metadata["sender"]["login"] - ): - lifecycle.record_failure(GitHubInstallationError.USER_MISMATCH) + if active_user_organization is None: return error( request, - self.active_user_organization, - error_short=GitHubInstallationError.USER_MISMATCH, + None, + error_short=GitHubInstallationError.MISSING_ORGANIZATION, + error_long=ERR_INTEGRATION_MISSING_ORGANIZATION, ) - return pipeline.next_step() + # The template flow also picks up installation_id from query params + if installation_id := request.GET.get("installation_id"): + pipeline.bind_state("installation_id", installation_id) - def check_pending_integration_deletion(self, request: HttpRequest) -> HttpResponse | None: - if self.active_user_organization is None: - return error( - request, - None, - error_short=GitHubInstallationError.MISSING_ORGANIZATION, - error_long=ERR_INTEGRATION_MISSING_ORGANIZATION, - ) + try: + resolved_id = validate_github_installation(pipeline) + except GitHubPipelineError as e: + lifecycle.record_failure(e.message) + return error( + request, active_user_organization, error_short=e.message, error_long=e.message + ) - # We want to wait until the scheduled deletions finish or else the - # post install to migrate repos do not work. - integration_pending_deletion_exists = OrganizationIntegration.objects.filter( - integration__provider=GitHubIntegrationProvider.key, - organization_id=self.active_user_organization.organization.id, - status=ObjectStatus.PENDING_DELETION, - ).exists() + if resolved_id is None: + return HttpResponseRedirect(get_install_app_url()) - if integration_pending_deletion_exists: - return error( - request, - self.active_user_organization, - error_short=GitHubInstallationError.PENDING_DELETION, - error_long=ERR_INTEGRATION_PENDING_DELETION, - ) - return None + return pipeline.next_step() diff --git a/src/sentry/integrations/github/utils.py b/src/sentry/integrations/github/utils.py index 6f4198481e24d3..a75284daff649c 100644 --- a/src/sentry/integrations/github/utils.py +++ b/src/sentry/integrations/github/utils.py @@ -16,7 +16,7 @@ def get_jwt(github_id: str | None = None, github_private_key: str | None = None) -> str: if github_id is None: - github_id = options.get("github-app.id") + github_id = str(options.get("github-app.id")) if github_private_key is None: github_private_key = options.get("github-app.private-key") exp_ = datetime.datetime.utcnow() + datetime.timedelta(minutes=10) diff --git a/src/sentry/integrations/github_copilot/client.py b/src/sentry/integrations/github_copilot/client.py index 586ec7fe2e52cb..99b32ddb741bbf 100644 --- a/src/sentry/integrations/github_copilot/client.py +++ b/src/sentry/integrations/github_copilot/client.py @@ -8,7 +8,6 @@ from sentry.integrations.github_copilot.models import ( GithubCopilotTask, GithubCopilotTaskRequest, - GithubCopilotTaskResponse, GithubPRFromGraphQL, ) from sentry.seer.autofix.utils import CodingAgentProviderType, CodingAgentState, CodingAgentStatus @@ -98,11 +97,7 @@ def launch(self, *, webhook_url: str, request: CodingAgentLaunchRequest) -> Codi }, ) - response_json = api_response.json - if isinstance(response_json, dict) and "task" in response_json: - task = GithubCopilotTaskResponse.validate(response_json).task - else: - task = GithubCopilotTask.validate(response_json) + task = GithubCopilotTask.validate(api_response.json) agent_id = self.encode_agent_id(owner, repo, task.id) diff --git a/src/sentry/integrations/github_copilot/models.py b/src/sentry/integrations/github_copilot/models.py index 8f952b0b29c51f..808499d5da1975 100644 --- a/src/sentry/integrations/github_copilot/models.py +++ b/src/sentry/integrations/github_copilot/models.py @@ -1,5 +1,7 @@ from __future__ import annotations +from typing import Any + from pydantic import BaseModel @@ -14,6 +16,18 @@ class GithubCopilotTaskRequest(BaseModel): event_type: str = "sentry" +class GithubCopilotUser(BaseModel): + """Lightweight user/owner reference returned by the GitHub Copilot API.""" + + id: int + + +class GithubCopilotRepository(BaseModel): + """Repository reference returned by the GitHub Copilot API.""" + + id: int + + class GithubCopilotArtifactData(BaseModel): """Data for an artifact - structure varies by type""" @@ -39,13 +53,14 @@ class GithubCopilotSession(BaseModel): id: str name: str | None = None state: str | None = None # queued, in_progress, completed, failed, timed_out - user_id: int | None = None + user: GithubCopilotUser | None = None + owner: GithubCopilotUser | None = None agent_id: int | None = None agent_type: str | None = None resource_type: str | None = None resource_id: int | None = None resource_global_id: str | None = None - last_updated_at: str | None = None + updated_at: str | None = None created_at: str | None = None completed_at: str | None = None event_type: str | None = None @@ -60,6 +75,7 @@ class GithubCopilotAgentCollaborator(BaseModel): agent_type: str | None = None agent_id: int | None = None agent_task_id: str | None = None + slug: str | None = None class GithubCopilotTask(BaseModel): @@ -67,28 +83,23 @@ class GithubCopilotTask(BaseModel): id: str name: str | None = None - creator_id: int | None = None - user_collaborators: list[int] | None = None + # creator/owner changed from int IDs to objects: {"id": int} + creator: GithubCopilotUser | None = None + owner: GithubCopilotUser | None = None + # user_collaborators is transitioning from list[int] to list[User objects]. + # Use list[Any] to accept both during the migration. + user_collaborators: list[Any] | None = None agent_collaborators: list[GithubCopilotAgentCollaborator] | None = None - owner_id: int | None = None - repo_id: int | None = None - status: str | None = None + repository: GithubCopilotRepository | None = None state: str | None = None # queued, in_progress, completed, failed, timed_out session_count: int | None = None artifacts: list[GithubCopilotArtifact] | None = None archived_at: str | None = None - last_updated_at: str | None = None + updated_at: str | None = None created_at: str | None = None sessions: list[GithubCopilotSession] | None = None - - -class GithubCopilotTaskResponse(BaseModel): - """ - Response from GitHub Copilot Tasks API. - The API wraps the task object in a {"task": {...}} envelope. - """ - - task: GithubCopilotTask + html_url: str | None = None + url: str | None = None class GithubPRFromGraphQL(BaseModel): diff --git a/src/sentry/integrations/gitlab/integration.py b/src/sentry/integrations/gitlab/integration.py index ae90f2dedd832a..292670e79e5bae 100644 --- a/src/sentry/integrations/gitlab/integration.py +++ b/src/sentry/integrations/gitlab/integration.py @@ -228,6 +228,30 @@ def _get_organization_config_default_values(self) -> list[dict[str, Any]]: "label": _("Sync Sentry Comments to GitLab"), "help": _("Post comments from Sentry issues to linked GitLab issues"), }, + { + "name": self.inbound_status_key, + "type": "boolean", + "label": _("Sync GitLab Status to Sentry"), + "help": _( + "When a GitLab issue is marked closed, resolve its linked issue in Sentry. " + "When a GitLab issue is reopened, unresolve its linked Sentry issue." + ), + "default": False, + }, + { + "name": self.resolution_strategy_key, + "label": "Resolve", + "type": "select", + "placeholder": "Resolve", + "choices": [ + ("resolve", "Resolve"), + ("resolve_current_release", "Resolve in Current Release"), + ("resolve_next_release", "Resolve in Next Release"), + ], + "help": _( + "Select what action to take on Sentry Issue when GitLab ticket is marked Closed." + ), + }, ] ) diff --git a/src/sentry/integrations/gitlab/issue_sync.py b/src/sentry/integrations/gitlab/issue_sync.py index 9f70220c8c62a2..6012b00ff16357 100644 --- a/src/sentry/integrations/gitlab/issue_sync.py +++ b/src/sentry/integrations/gitlab/issue_sync.py @@ -6,6 +6,7 @@ from urllib.parse import quote from sentry import features +from sentry.integrations.gitlab.types import GitLabIssueAction from sentry.integrations.mixins.issues import IssueSyncIntegration, ResolveSyncAction from sentry.integrations.models.external_actor import ExternalActor from sentry.integrations.models.external_issue import ExternalIssue @@ -22,6 +23,8 @@ class GitlabIssueSyncSpec(IssueSyncIntegration): comment_key = "sync_comments" outbound_assignee_key = "sync_forward_assignment" inbound_assignee_key = "sync_reverse_assignment" + inbound_status_key = "sync_status_reverse" + resolution_strategy_key = "resolution_strategy" def check_feature_flag(self) -> bool: """ @@ -169,6 +172,16 @@ def get_resolve_sync_action(self, data: Mapping[str, Any]) -> ResolveSyncAction: Given webhook data, check whether the GitLab issue status changed. GitLab issues have opened/closed state. """ + if not self.check_feature_flag(): + return ResolveSyncAction.NOOP + + action = data.get("action") + + if action == GitLabIssueAction.CLOSE: + return ResolveSyncAction.RESOLVE + elif action == GitLabIssueAction.REOPEN: + return ResolveSyncAction.UNRESOLVE + return ResolveSyncAction.NOOP def get_config_data(self): diff --git a/src/sentry/integrations/gitlab/types.py b/src/sentry/integrations/gitlab/types.py index 3182bef305017d..34935f78d6972a 100644 --- a/src/sentry/integrations/gitlab/types.py +++ b/src/sentry/integrations/gitlab/types.py @@ -1,20 +1,11 @@ from enum import StrEnum -class GitLabIssueStatus(StrEnum): - OPENED = "opened" - CLOSED = "closed" - - @classmethod - def get_choices(cls): - """Return choices formatted for dropdown selectors""" - return [(status.value, status.value.capitalize()) for status in cls] - - class GitLabIssueAction(StrEnum): UPDATE = "update" OPEN = "open" REOPEN = "reopen" + CLOSE = "close" @classmethod def values(cls): diff --git a/src/sentry/integrations/gitlab/webhooks.py b/src/sentry/integrations/gitlab/webhooks.py index 76edeea09b0bad..39781c8f2ee3a6 100644 --- a/src/sentry/integrations/gitlab/webhooks.py +++ b/src/sentry/integrations/gitlab/webhooks.py @@ -17,8 +17,10 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import Endpoint, cell_silo_endpoint +from sentry.constants import ObjectStatus from sentry.integrations.base import IntegrationDomain from sentry.integrations.gitlab.types import GitLabIssueAction +from sentry.integrations.mixins.issues import IssueSyncIntegration from sentry.integrations.services.integration import integration_service from sentry.integrations.services.integration.model import RpcIntegration from sentry.integrations.source_code_management.webhook import SCMWebhook @@ -129,6 +131,7 @@ def event_type(self) -> IntegrationWebhookEventType: def __call__(self, event: Mapping[str, Any], **kwargs): if not (integration := kwargs.get("integration")): raise ValueError("Integration must be provided") + organization: RpcOrganization | None = kwargs.get("organization") external_issue_key = self._extract_issue_key(event, integration) if not external_issue_key: @@ -144,10 +147,14 @@ def __call__(self, event: Mapping[str, Any], **kwargs): object_attributes = event.get("object_attributes", {}) action = object_attributes.get("action") - # Handle assignment changes - if action in GitLabIssueAction.values(): + # Handle assignment changes — CLOSE does not affect assignment + if action in GitLabIssueAction.values() and action != GitLabIssueAction.CLOSE: self._handle_assignment(integration, event, external_issue_key) + # Handle status changes (CLOSE and REOPEN) + if action in [GitLabIssueAction.CLOSE, GitLabIssueAction.REOPEN] and organization: + self._handle_status_change(integration, external_issue_key, action, organization.id) + def _handle_assignment( self, integration: RpcIntegration, @@ -216,6 +223,40 @@ def _handle_assignment( }, ) + def _handle_status_change( + self, + integration: RpcIntegration, + external_issue_key: str, + action: str, + organization_id: int, + ) -> None: + """ + Handle issue status changes (close/reopen). + + Triggers the sync_status_inbound task to update linked Sentry issues. + """ + org_integrations = integration_service.get_organization_integrations( + integration_id=integration.id, + organization_id=organization_id, + providers=[integration.provider], + status=ObjectStatus.ACTIVE, + ) + for org_integration in org_integrations: + installation = integration.get_installation(org_integration.organization_id) + if isinstance(installation, IssueSyncIntegration): + installation.sync_status_inbound( + external_issue_key, + {"action": action}, + ) + logger.info( + "gitlab.webhook.status.synced", + extra={ + "integration_id": integration.id, + "external_issue_key": external_issue_key, + "action": action, + }, + ) + def _extract_issue_key( self, event: Mapping[str, Any], integration: RpcIntegration ) -> str | None: diff --git a/src/sentry/integrations/messaging/metrics.py b/src/sentry/integrations/messaging/metrics.py index d0568446953aa5..b8bf47f8d87ac7 100644 --- a/src/sentry/integrations/messaging/metrics.py +++ b/src/sentry/integrations/messaging/metrics.py @@ -118,4 +118,4 @@ class AppMentionHaltReason(StrEnum): ORGANIZATION_NOT_FOUND = "organization_not_found" ORGANIZATION_NOT_ACTIVE = "organization_not_active" FEATURE_NOT_ENABLED = "feature_not_enabled" - MISSING_CHANNEL_OR_TEXT = "missing_channel_or_text" + MISSING_EVENT_DATA = "missing_event_data" diff --git a/src/sentry/integrations/slack/integration.py b/src/sentry/integrations/slack/integration.py index 0533c404892d3a..81a1106abc23cd 100644 --- a/src/sentry/integrations/slack/integration.py +++ b/src/sentry/integrations/slack/integration.py @@ -3,7 +3,7 @@ import logging from collections import namedtuple from collections.abc import Mapping, Sequence -from typing import Any +from typing import Any, Optional from django.utils.translation import gettext_lazy as _ from slack_sdk import WebClient @@ -251,10 +251,13 @@ def set_thread_status( channel_id: str, thread_ts: str, status: str, + loading_messages: Optional[list[str]] = None, ) -> None: """ Set a status indicator in a Slack assistant thread (e.g. "Thinking..."). The status auto-clears when the bot sends a reply, or after 2 minutes. + + Sending an empty status message will clear the status indicator. """ client = self.get_client() try: @@ -262,6 +265,7 @@ def set_thread_status( channel_id=channel_id, thread_ts=thread_ts, status=status, + loading_messages=loading_messages, ) except SlackApiError: _logger.warning( diff --git a/src/sentry/integrations/slack/webhooks/action.py b/src/sentry/integrations/slack/webhooks/action.py index 7c3090b1cb1bbe..259759504f5187 100644 --- a/src/sentry/integrations/slack/webhooks/action.py +++ b/src/sentry/integrations/slack/webhooks/action.py @@ -660,7 +660,7 @@ def get_action_list(cls, slack_request: SlackActionRequest) -> list[BlockKitMess name=action_name, label=action_data["text"]["text"], type=action_data["type"], - value=action_data["value"], + value=action_data.get("value", ""), action_id=action_data["action_id"], block_id=action_data["block_id"], ) diff --git a/src/sentry/integrations/slack/webhooks/event.py b/src/sentry/integrations/slack/webhooks/event.py index f02495e4421200..b6b347df46b844 100644 --- a/src/sentry/integrations/slack/webhooks/event.py +++ b/src/sentry/integrations/slack/webhooks/event.py @@ -7,6 +7,7 @@ import orjson import sentry_sdk +from rest_framework.exceptions import NotFound from rest_framework.request import Request from rest_framework.response import Response from slack_sdk.errors import SlackApiError @@ -23,6 +24,7 @@ ) from sentry.integrations.services.integration import integration_service from sentry.integrations.slack.analytics import SlackIntegrationChartUnfurl +from sentry.integrations.slack.integration import SlackIntegration from sentry.integrations.slack.message_builder.help import SlackHelpMessageBuilder from sentry.integrations.slack.message_builder.prompt import SlackPromptLinkMessageBuilder from sentry.integrations.slack.requests.base import SlackDMRequest, SlackRequestError @@ -334,47 +336,86 @@ def on_app_mention(self, slack_request: SlackDMRequest) -> Response: organization_id = ois[0].organization_id lifecycle.add_extra("organization_id", organization_id) - organization_context = organization_service.get_organization_by_id( - id=organization_id, - user_id=None, - include_projects=False, - include_teams=False, + installation = slack_request.integration.get_installation( + organization_id=organization_id ) - if not organization_context: + assert isinstance(installation, SlackIntegration) + try: + organization = installation.organization + except NotFound: lifecycle.record_halt(AppMentionHaltReason.ORGANIZATION_NOT_FOUND) return self.respond() - if organization_context.organization.status != OrganizationStatus.ACTIVE: - lifecycle.add_extra("status", organization_context.organization.status) + if organization.status != OrganizationStatus.ACTIVE: + lifecycle.add_extra("status", organization.status) lifecycle.record_halt(AppMentionHaltReason.ORGANIZATION_NOT_ACTIVE) return self.respond() - if not features.has( - "organizations:seer-slack-explorer", organization_context.organization - ): + if not features.has("organizations:seer-slack-explorer", organization): lifecycle.record_halt(AppMentionHaltReason.FEATURE_NOT_ENABLED) return self.respond() channel_id = data.get("channel") text = data.get("text") - thread_ts = data.get("ts") + ts = data.get("ts") + thread_ts = data.get("thread_ts") # None for top-level messages - if not channel_id or not text or not thread_ts: - lifecycle.record_halt(AppMentionHaltReason.MISSING_CHANNEL_OR_TEXT) + lifecycle.add_extras( + { + "channel_id": channel_id, + "text": text, + "ts": ts, + "thread_ts": thread_ts, + "user_id": slack_request.user_id, + } + ) + + if not channel_id or not text or not ts or not slack_request.user_id: + lifecycle.record_halt(AppMentionHaltReason.MISSING_EVENT_DATA) return self.respond() + try: + installation.set_thread_status( + channel_id=channel_id, + thread_ts=thread_ts or ts, + status="Thinking...", + loading_messages=[ + "Digging through your errors...", + "Sifting through stack traces...", + "Blaming the right code...", + "Following the breadcrumbs...", + "Asking the stack trace nicely...", + "Reading between the stack frames...", + "Hold on, I've seen this one before...", + "It worked on my machine...", + ], + ) + except Exception: + _logger.exception( + "slack.assistant_threads_setStatus.failed", + extra={ + "integration_id": slack_request.integration.id, + "channel_id": channel_id, + "thread_ts": thread_ts or ts, + }, + ) + + authorizations = slack_request.data.get("authorizations") or [] + bot_user_id = authorizations[0].get("user_id", "") if authorizations else "" + process_mention_for_slack.apply_async( kwargs={ "integration_id": slack_request.integration.id, "organization_id": organization_id, "channel_id": channel_id, + "ts": ts, "thread_ts": thread_ts, "text": text, "slack_user_id": slack_request.user_id, + "bot_user_id": bot_user_id, } ) - - return self.respond() + return self.respond() # TODO(dcramer): implement app_uninstalled and tokens_revoked def post(self, request: Request) -> Response: diff --git a/src/sentry/issues/endpoints/group_similar_issues_embeddings.py b/src/sentry/issues/endpoints/group_similar_issues_embeddings.py index f3d4921e7e431d..ce678207fc0a5f 100644 --- a/src/sentry/issues/endpoints/group_similar_issues_embeddings.py +++ b/src/sentry/issues/endpoints/group_similar_issues_embeddings.py @@ -6,7 +6,7 @@ from rest_framework.request import Request from rest_framework.response import Response -from sentry import analytics, options +from sentry import analytics from sentry.api.analytics import GroupSimilarIssuesEmbeddingsCountEvent from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus @@ -123,7 +123,6 @@ def get(self, request: Request, group: Group) -> Response: "exception_type": get_path(latest_event.data, "exception", "values", -1, "type"), "read_only": True, "referrer": "similar_issues", - "use_reranking": options.get("seer.similarity.similar_issues.use_reranking"), "model": model_version, "training_mode": False, "platform": latest_event.platform or "unknown", @@ -134,10 +133,6 @@ def get(self, request: Request, group: Group) -> Response: if request.GET.get("threshold"): similar_issues_params["threshold"] = float(request.GET["threshold"]) - # Override `use_reranking` value if necessary - if request.GET.get("useReranking"): - similar_issues_params["use_reranking"] = request.GET["useReranking"] == "true" - logger.info("Similar issues embeddings parameters", extra=similar_issues_params) viewer_context = SeerViewerContext( diff --git a/src/sentry/middleware/integrations/tasks.py b/src/sentry/middleware/integrations/tasks.py index 65cec4735c816f..416dff03ec6668 100644 --- a/src/sentry/middleware/integrations/tasks.py +++ b/src/sentry/middleware/integrations/tasks.py @@ -121,11 +121,12 @@ def unpack_payload(self, response: Response) -> Any: silo_mode=SiloMode.CONTROL, ) def convert_to_async_slack_response( - region_names: list[str], payload: dict[str, Any], response_url: str, + cell_names: list[str] | None = None, # TODO(cells): make required once region_names is removed + region_names: list[str] | None = None, # TODO(cells): remove after queue drains ) -> None: - _AsyncSlackDispatcher(payload, response_url).dispatch(region_names) + _AsyncSlackDispatcher(payload, response_url).dispatch(cell_names or region_names or []) class _AsyncDiscordDispatcher(_AsyncCellDispatcher): @@ -148,9 +149,10 @@ def unpack_payload(self, response: Response) -> Any: silo_mode=SiloMode.CONTROL, ) def convert_to_async_discord_response( - region_names: list[str], payload: dict[str, Any], response_url: str, + cell_names: list[str] | None = None, # TODO(cells): make required once region_names is removed + region_names: list[str] | None = None, # TODO(cells): remove after queue drains ) -> None: """ This task asks relevant cell silos for response data to send asynchronously to Discord. It @@ -159,7 +161,9 @@ def convert_to_async_discord_response( In the event this task finishes prior to returning the above type, the outbound post will fail. """ - response = _AsyncDiscordDispatcher(payload, response_url).dispatch(region_names) + response = _AsyncDiscordDispatcher(payload, response_url).dispatch( + cell_names or region_names or [] + ) if response is not None and response.status_code == status.HTTP_404_NOT_FOUND: raise Exception("Discord hook is not ready.") diff --git a/src/sentry/monitors/tasks/clock_pulse.py b/src/sentry/monitors/tasks/clock_pulse.py index 9281eed4b0e69d..dc42bdcfb4eb1c 100644 --- a/src/sentry/monitors/tasks/clock_pulse.py +++ b/src/sentry/monitors/tasks/clock_pulse.py @@ -8,7 +8,10 @@ from arroyo import Partition from arroyo import Topic as ArroyoTopic from arroyo.backends.kafka import KafkaPayload -from confluent_kafka.admin import AdminClient, PartitionMetadata +from confluent_kafka.admin import ( # type: ignore[attr-defined] + AdminClient, + PartitionMetadata, +) from django.conf import settings from sentry_kafka_schemas.codecs import Codec from sentry_kafka_schemas.schema_types.ingest_monitors_v1 import ClockPulse, IngestMonitorMessage diff --git a/src/sentry/options/defaults.py b/src/sentry/options/defaults.py index a3f6df779157af..12942114b0c17e 100644 --- a/src/sentry/options/defaults.py +++ b/src/sentry/options/defaults.py @@ -1247,20 +1247,6 @@ flags=FLAG_AUTOMATOR_MODIFIABLE, ) -register( - "seer.similarity.ingest.use_reranking", - type=Bool, - default=True, - flags=FLAG_AUTOMATOR_MODIFIABLE, -) - -register( - "seer.similarity.similar_issues.use_reranking", - type=Bool, - default=True, - flags=FLAG_AUTOMATOR_MODIFIABLE, -) - register( "seer.similarity.ingest.num_matches_to_request", type=Int, @@ -1335,14 +1321,6 @@ flags=FLAG_ALLOW_EMPTY | FLAG_AUTOMATOR_MODIFIABLE, ) -# seer embeddings backfill batch size -register( - "embeddings-grouping.seer.backfill-batch-size", - type=Int, - default=10, - flags=FLAG_ALLOW_EMPTY | FLAG_AUTOMATOR_MODIFIABLE, -) - register( "embeddings-grouping.seer.delete-record-batch-size", type=Int, @@ -2619,6 +2597,29 @@ flags=FLAG_AUTOMATOR_MODIFIABLE, ) +# Circuit breaker configuration for webhook endpoint failure detection. +# Keys match RateBasedTripStrategyConfig + CircuitBreakerConfig +register( + "sentry-apps.webhook.circuit-breaker.config", + type=Dict, + default={ + "error_limit_window": 600, # 10 minutes + "broken_state_duration": 300, # 5 minutes + "threshold": 0.5, # 50% error rate + "floor": 500, # 500 errors before error rate check applies + "metrics_key": "sentry-app.webhook", # to avoid high cardinality slug tag + }, + flags=FLAG_AUTOMATOR_MODIFIABLE, +) + +# When True, the circuit breaker tracks state and emits metrics but does not block requests. +register( + "sentry-apps.webhook.circuit-breaker.dry-run", + type=Bool, + default=False, + flags=FLAG_AUTOMATOR_MODIFIABLE, +) + # Enables statistical detectors for a project register( "statistical_detectors.enable", @@ -3441,58 +3442,16 @@ flags=FLAG_AUTOMATOR_MODIFIABLE, ) -register( - "similarity.backfill_nodestore_use_multithread", - default=False, - flags=FLAG_AUTOMATOR_MODIFIABLE, -) - -register( - "similarity.backfill_nodestore_chunk_size", - default=5, - flags=FLAG_AUTOMATOR_MODIFIABLE, -) - -register( - "similarity.backfill_nodestore_threads", - default=6, - flags=FLAG_AUTOMATOR_MODIFIABLE, -) -register( - "similarity.backfill_snuba_concurrent_requests", - default=20, - flags=FLAG_AUTOMATOR_MODIFIABLE, -) -register( - "similarity.backfill_seer_chunk_size", - default=30, - flags=FLAG_AUTOMATOR_MODIFIABLE, -) -register( - "similarity.backfill_seer_threads", - default=1, - flags=FLAG_AUTOMATOR_MODIFIABLE, -) register( "similarity.backfill_project_cohort_size", default=1000, flags=FLAG_AUTOMATOR_MODIFIABLE, ) -register( - "similarity.backfill_total_worker_count", - default=6, - flags=FLAG_AUTOMATOR_MODIFIABLE, -) register( "similarity.new_project_seer_grouping.enabled", default=False, flags=FLAG_AUTOMATOR_MODIFIABLE, ) -register( - "similarity.backfill_use_reranking", - default=False, - flags=FLAG_AUTOMATOR_MODIFIABLE, -) register( "delayed_processing.batch_size", default=10000, diff --git a/src/sentry/preprod/size_analysis/grouptype.py b/src/sentry/preprod/size_analysis/grouptype.py index ce89fd2b18864a..8e0b30126029e2 100644 --- a/src/sentry/preprod/size_analysis/grouptype.py +++ b/src/sentry/preprod/size_analysis/grouptype.py @@ -295,6 +295,11 @@ def create_occurrence( evidence_data: dict[str, Any] = { "detector_id": self.detector.id, + "value": self.extract_value(data_packet), + "conditions": [ + result.condition.get_snapshot() for result in evaluation_result.condition_results + ], + "config": self.detector.config, } if metadata: evidence_data["head_artifact_id"] = metadata["head_artifact_id"] diff --git a/src/sentry/preprod/tasks.py b/src/sentry/preprod/tasks.py index ca25645895dd90..493a03ddac08a2 100644 --- a/src/sentry/preprod/tasks.py +++ b/src/sentry/preprod/tasks.py @@ -11,6 +11,7 @@ from django.utils import timezone from taskbroker_client.retry import Retry +from sentry import features from sentry.constants import DataCategory from sentry.models.commitcomparison import CommitComparison from sentry.models.organization import Organization @@ -28,7 +29,10 @@ PreprodBuildConfiguration, ) from sentry.preprod.producer import PreprodFeature, produce_preprod_artifact_to_kafka -from sentry.preprod.quotas import has_installable_quota, has_size_quota +from sentry.preprod.quotas import ( + has_installable_quota, + has_size_quota, +) from sentry.preprod.size_analysis.models import SizeAnalysisResults from sentry.preprod.size_analysis.tasks import ( compare_preprod_artifact_size_analysis, @@ -47,7 +51,7 @@ set_assemble_status, ) from sentry.tasks.base import instrumented_task -from sentry.taskworker.namespaces import preprod_tasks +from sentry.taskworker.namespaces import launchpad_tasks, preprod_tasks from sentry.utils import metrics from sentry.utils.outcomes import Outcome, track_outcome from sentry.utils.sdk import bind_organization_context @@ -55,6 +59,17 @@ logger = logging.getLogger(__name__) +# Typed RPC stub — the body is never called. The decorator registers the task signature +# with the external namespace; dispatch happens via process_artifact.apply_async(). +@launchpad_tasks.register( + name="process_artifact", + retry=Retry(times=3), + processing_deadline_duration=60 * 12, +) +def process_artifact(artifact_id: str, project_id: str, organization_id: str) -> None: + pass + + @instrumented_task( name="sentry.preprod.tasks.assemble_preprod_artifact", retry=Retry(times=3), @@ -83,6 +98,7 @@ def assemble_preprod_artifact( }, ) + assemble_result = None try: organization = Organization.objects.get_from_cache(pk=org_id) project = Project.objects.get(id=project_id, organization=organization) @@ -142,43 +158,18 @@ def assemble_preprod_artifact( ) return + finally: + try: + if assemble_result is not None: + assemble_result.bundle_temp_file.close() + except Exception: + pass - try: - # Note: requested_features is no longer used for filtering - all features are - # requested here, and the actual quota/filter checks happen in the update endpoint - # (project_preprod_artifact_update.py) after preprocessing completes. - produce_preprod_artifact_to_kafka( - project_id=project_id, - organization_id=org_id, - artifact_id=artifact_id, - requested_features=[ - PreprodFeature.SIZE_ANALYSIS, - PreprodFeature.BUILD_DISTRIBUTION, - ], - ) - except Exception as e: - user_friendly_error_message = "Failed to dispatch preprod artifact event for analysis" - sentry_sdk.capture_exception(e) - logger.exception( - user_friendly_error_message, - extra={ - "project_id": project_id, - "organization_id": org_id, - "checksum": checksum, - "preprod_artifact_id": artifact_id, - }, - ) - PreprodArtifact.objects.filter(id=artifact_id).update( - state=PreprodArtifact.ArtifactState.FAILED, - error_code=PreprodArtifact.ErrorCode.ARTIFACT_PROCESSING_ERROR, - error_message=user_friendly_error_message, - ) - create_preprod_status_check_task.apply_async( - kwargs={ - "preprod_artifact_id": artifact_id, - "caller": "assemble_dispatch_error", - } - ) + if features.has("organizations:launchpad-taskbroker-rollout", organization): + _dispatch_taskbroker_shadow(project_id, org_id, artifact_id) + + kafka_dispatched = _dispatch_kafka(project_id, org_id, artifact_id, checksum) + if not kafka_dispatched: return logger.info( @@ -979,3 +970,74 @@ def detect_expired_preprod_artifacts() -> None: + expired_size_comparisons_count, }, ) + + +def _dispatch_kafka(project_id: int, org_id: int, artifact_id: int, checksum: str) -> bool: + # Note: requested_features is no longer used for filtering - all features are + # requested here, and the actual quota/filter checks happen in the update endpoint + # (project_preprod_artifact_update.py) after preprocessing completes. + try: + produce_preprod_artifact_to_kafka( + project_id=project_id, + organization_id=org_id, + artifact_id=artifact_id, + requested_features=[ + PreprodFeature.SIZE_ANALYSIS, + PreprodFeature.BUILD_DISTRIBUTION, + ], + ) + return True + except Exception as e: + user_friendly_error_message = "Failed to dispatch preprod artifact event for analysis" + sentry_sdk.capture_exception(e) + logger.exception( + user_friendly_error_message, + extra={ + "project_id": project_id, + "organization_id": org_id, + "checksum": checksum, + "preprod_artifact_id": artifact_id, + }, + ) + PreprodArtifact.objects.filter(id=artifact_id).update( + state=PreprodArtifact.ArtifactState.FAILED, + error_code=PreprodArtifact.ErrorCode.ARTIFACT_PROCESSING_ERROR, + error_message=user_friendly_error_message, + ) + create_preprod_status_check_task.apply_async( + kwargs={ + "preprod_artifact_id": artifact_id, + "caller": "assemble_dispatch_error", + } + ) + return False + + +def _dispatch_taskbroker_shadow(project_id: int, org_id: int, artifact_id: int) -> None: + # TODO: When taskbroker becomes the primary path, add PreprodArtifactSizeMetrics + # state management here (mirroring project_preprod_artifact_update.py). Currently + # omitted to avoid racing with the primary Kafka consumer path. + try: + logger.info( + "preprod.dispatch_taskbroker_shadow", + extra={ + "project_id": project_id, + "organization_id": org_id, + "preprod_artifact_id": artifact_id, + }, + ) + + process_artifact.delay( + artifact_id=str(artifact_id), + project_id=str(project_id), + organization_id=str(org_id), + ) + except Exception: + logger.exception( + "Failed to dispatch shadow taskbroker event", + extra={ + "project_id": project_id, + "organization_id": org_id, + "preprod_artifact_id": artifact_id, + }, + ) diff --git a/src/sentry/seer/endpoints/seer_rpc.py b/src/sentry/seer/endpoints/seer_rpc.py index 2795c05760c5eb..f6c034d673e24b 100644 --- a/src/sentry/seer/endpoints/seer_rpc.py +++ b/src/sentry/seer/endpoints/seer_rpc.py @@ -663,6 +663,61 @@ def validate_repo( return {"valid": True, "integration_id": repo.integration_id} +def get_repo_installation_id( + *, + organization_id: int, + provider: str, + external_id: str, + owner: str, + name: str, +) -> dict[str, Any]: + """ + Look up a repository and return the external_id of its associated integration (the installation ID). + + Args: + organization_id: The Sentry organization ID + provider: The SCM provider (e.g., "github", "github_enterprise") + external_id: The repository's external ID in the provider's system + owner: The repository owner (e.g., "getsentry") + name: The repository name (e.g., "sentry") + + Returns: + {"installation_id": } if found + {"error": } if not found or unsupported + """ + repo = filter_repo_by_provider(organization_id, provider, external_id, owner, name).first() + + if not repo: + return {"error": "repository_not_found"} + + if repo.provider not in SEER_SUPPORTED_SCM_PROVIDERS: + return {"error": "unsupported_provider"} + + if repo.integration_id is None: + return {"error": "no_integration"} + + integration = integration_service.get_integration(integration_id=repo.integration_id) + if integration is None: + return {"error": "integration_not_found"} + + # GitHub stores the installation ID as the integration's external_id, + # while GitHub Enterprise stores it in metadata["installation_id"]. + if integration.provider == IntegrationProviderSlug.GITHUB_ENTERPRISE.value: + installation_id = integration.metadata.get("installation_id") + elif integration.provider == IntegrationProviderSlug.GITHUB.value: + installation_id = integration.external_id + else: + return {"error": "unsupported_provider"} + + if not installation_id: + return {"error": "installation_id_not_found"} + + return { + "installation_id": installation_id, + "permissions": integration.metadata.get("permissions"), + } + + def check_repository_integrations_status(*, repository_integrations: list[dict[str, Any]]) -> dict: """ Check whether repository integrations exist and are active. @@ -760,6 +815,7 @@ def check_repository_integrations_status(*, repository_integrations: list[dict[s "get_organization_project_ids": get_organization_project_ids, "check_repository_integrations_status": check_repository_integrations_status, "validate_repo": validate_repo, + "get_repo_installation_id": get_repo_installation_id, # # Autofix "get_organization_slug": get_organization_slug, diff --git a/src/sentry/seer/entrypoints/metrics.py b/src/sentry/seer/entrypoints/metrics.py index abe75ca3b8c311..f71d6a058ccb1f 100644 --- a/src/sentry/seer/entrypoints/metrics.py +++ b/src/sentry/seer/entrypoints/metrics.py @@ -49,6 +49,7 @@ class SlackEntrypointInteractionType(StrEnum): PROCESS_THREAD_UPDATE = "process_thread_update" SCHEDULE_ALL_THREAD_UPDATES = "schedule_all_thread_updates" UPDATE_EXISTING_MESSAGE = "update_existing_message" + PROCESS_MENTION = "process_mention" @dataclass diff --git a/src/sentry/seer/entrypoints/operator.py b/src/sentry/seer/entrypoints/operator.py index 0d1e3062633d4c..0bcf26d79213b9 100644 --- a/src/sentry/seer/entrypoints/operator.py +++ b/src/sentry/seer/entrypoints/operator.py @@ -785,7 +785,7 @@ def execute(cls, organization: Organization, run_id: int) -> None: } ) - summary = "Explorer result could not be fetched. Please try again." + summary: str | None = None try: state = fetch_run_status(run_id, organization) for block in reversed(state.blocks): diff --git a/src/sentry/seer/entrypoints/slack/entrypoint.py b/src/sentry/seer/entrypoints/slack/entrypoint.py index 348cb08a8cc9ca..f51dabaa6746fd 100644 --- a/src/sentry/seer/entrypoints/slack/entrypoint.py +++ b/src/sentry/seer/entrypoints/slack/entrypoint.py @@ -42,6 +42,12 @@ from sentry.models.group import Group +class EntrypointSetupError(Exception): + """Raised when entrypoint construction fails during mention processing.""" + + pass + + class SlackThreadDetails(TypedDict): thread_ts: str channel_id: str @@ -60,7 +66,6 @@ class SlackExplorerCachePayload(TypedDict): organization_id: int integration_id: int thread: SlackThreadDetails - message_ts: str class SlackAutofixEntrypoint( @@ -339,8 +344,7 @@ def __init__( integration_id: int, organization_id: int, channel_id: str, - message_ts: str, - thread_ts: str | None, + thread_ts: str, slack_user_id: str, ): from sentry.integrations.services.integration import integration_service @@ -354,7 +358,7 @@ def __init__( status=ObjectStatus.ACTIVE, ) if not integration: - raise ValueError(f"Slack integration {integration_id} not found") + raise EntrypointSetupError(f"Slack integration {integration_id} not found") ois = integration_service.get_organization_integrations( integration_id=integration_id, @@ -363,15 +367,15 @@ def __init__( limit=1, ) if not ois: - raise ValueError( + raise EntrypointSetupError( f"Slack integration {integration_id} is not active for org {organization_id}" ) self.channel_id = channel_id - self.message_ts = message_ts - self.thread_ts = thread_ts or message_ts + self.thread_ts = thread_ts self.thread = SlackThreadDetails(thread_ts=self.thread_ts, channel_id=channel_id) self.organization_id = organization_id + self.integration = integration self.install = SlackIntegration(model=integration, organization_id=organization_id) self.slack_user_id = slack_user_id @@ -399,7 +403,6 @@ def create_explorer_cache_payload(self) -> SlackExplorerCachePayload: thread=self.thread, organization_id=self.organization_id, integration_id=self.install.model.id, - message_ts=self.message_ts, ) @staticmethod diff --git a/src/sentry/seer/entrypoints/slack/mention.py b/src/sentry/seer/entrypoints/slack/mention.py index 8531c2c5e8bf8a..0531283b14a20c 100644 --- a/src/sentry/seer/entrypoints/slack/mention.py +++ b/src/sentry/seer/entrypoints/slack/mention.py @@ -48,6 +48,74 @@ def extract_issue_links(text: str) -> list[IssueLink]: return results +def _extract_rich_text_element_text(element: Mapping[str, Any]) -> str: + """Extract text from a single rich_text sub-element (text, link, user, channel, etc.).""" + elem_type = element.get("type") + if elem_type == "text": + return element.get("text", "") + if elem_type == "link": + url = element.get("url", "") + label = element.get("text", "") + return f"<{url}|{label}>" if label else f"<{url}>" + if elem_type == "user": + return f"<@{element.get('user_id', '')}>" + if elem_type == "channel": + return f"<#{element.get('channel_id', '')}>" + if elem_type == "emoji": + return f":{element.get('name', '')}:" + if elem_type == "broadcast": + return f"@{element.get('range', 'here')}" + return "" + + +def _extract_block_text(block: Mapping[str, Any]) -> str: + """Extract readable text from a single Slack Block Kit block.""" + block_type = block.get("type") + + if block_type == "rich_text": + parts: list[str] = [] + for container in block.get("elements", []): + if not isinstance(container, dict): + continue + container_parts = [ + _extract_rich_text_element_text(el) for el in container.get("elements", []) + ] + parts.append("".join(container_parts)) + return "\n".join(parts) + + if block_type == "section": + section_parts: list[str] = [] + text_obj = block.get("text") + if isinstance(text_obj, dict): + section_parts.append(text_obj.get("text", "")) + for field in block.get("fields", []): + if isinstance(field, dict): + section_parts.append(field.get("text", "")) + return "\n".join(part for part in section_parts if part) + + if block_type == "context": + return " ".join( + el.get("text", "") + for el in block.get("elements", []) + if isinstance(el, dict) and el.get("type") in ("mrkdwn", "plain_text") + ) + + if block_type in ("header", "markdown"): + text_obj = block.get("text") + if isinstance(text_obj, dict): + return text_obj.get("text", "") + if isinstance(text_obj, str): + return text_obj + + return "" + + +def _extract_text_from_blocks(blocks: Sequence[Mapping[str, Any]]) -> str: + """Extract readable text from a list of Slack Block Kit blocks.""" + parts = [_extract_block_text(block) for block in blocks] + return "\n".join(part for part in parts if part) + + def build_thread_context(messages: Sequence[Mapping[str, Any]]) -> str: """Build a context string from thread history for Seer Explorer.""" if not messages: @@ -56,7 +124,15 @@ def build_thread_context(messages: Sequence[Mapping[str, Any]]) -> str: parts: list[str] = [] for msg in messages: user = msg.get("user", "unknown") - text = msg.get("text", "") + + text = "" + blocks = msg.get("blocks") + if blocks: + text = _extract_text_from_blocks(blocks) + + if not text: + text = msg.get("text", "") + if not text: continue parts.append(f"<@{user}>: {text}") diff --git a/src/sentry/seer/entrypoints/slack/metrics.py b/src/sentry/seer/entrypoints/slack/metrics.py new file mode 100644 index 00000000000000..464306f4bc119d --- /dev/null +++ b/src/sentry/seer/entrypoints/slack/metrics.py @@ -0,0 +1,13 @@ +from __future__ import annotations + +from enum import StrEnum + + +class ProcessMentionHaltReason(StrEnum): + IDENTITY_NOT_LINKED = "identity_not_linked" + USER_NOT_ORG_MEMBER = "user_not_org_member" + + +class ProcessMentionFailureReason(StrEnum): + ORG_NOT_FOUND = "org_not_found" + NO_EXPLORER_ACCESS = "no_explorer_access" diff --git a/src/sentry/seer/entrypoints/slack/tasks.py b/src/sentry/seer/entrypoints/slack/tasks.py index 28af4faa10331b..9151568e9fc661 100644 --- a/src/sentry/seer/entrypoints/slack/tasks.py +++ b/src/sentry/seer/entrypoints/slack/tasks.py @@ -2,10 +2,29 @@ import logging +from slack_sdk.models.blocks import ActionsBlock, ButtonElement, LinkButtonElement, MarkdownBlock from taskbroker_client.retry import Retry +from sentry.identity.services.identity import identity_service +from sentry.integrations.slack.views.link_identity import build_linking_url +from sentry.integrations.types import IntegrationProviderSlug +from sentry.models.organization import Organization +from sentry.notifications.platform.slack.provider import SlackRenderable +from sentry.seer.entrypoints.metrics import ( + SlackEntrypointEventLifecycleMetric, + SlackEntrypointInteractionType, +) +from sentry.seer.entrypoints.operator import SeerExplorerOperator +from sentry.seer.entrypoints.slack.entrypoint import EntrypointSetupError, SlackExplorerEntrypoint +from sentry.seer.entrypoints.slack.mention import build_thread_context, extract_prompt +from sentry.seer.entrypoints.slack.metrics import ( + ProcessMentionFailureReason, + ProcessMentionHaltReason, +) from sentry.tasks.base import instrumented_task from sentry.taskworker.namespaces import integrations_tasks +from sentry.users.services.user import RpcUser +from sentry.users.services.user.service import user_service logger = logging.getLogger(__name__) @@ -21,25 +40,194 @@ def process_mention_for_slack( integration_id: int, organization_id: int, channel_id: str, - thread_ts: str, + ts: str, + thread_ts: str | None = None, text: str, slack_user_id: str, + bot_user_id: str, ) -> None: """ Process a Slack @mention for Seer Explorer. - Parses the mention, extracts issue links and thread context, - and triggers an Explorer run. + Parses the mention, extracts thread context, + and triggers an Explorer run via SeerExplorerOperator. - TODO(ISWF-2023): Implement full processing logic. + ``ts`` is the message's own timestamp (always present). + ``thread_ts`` is the parent thread's timestamp (None for top-level messages). + + Authorization: Access is gated by the org-level ``seer-slack-workflows`` + feature flag and ``has_explorer_access()``. The incoming webhook is + verified by ``SlackDMRequest.validate()``. The Slack user must have a + linked Sentry identity; if not, an ephemeral prompt to link is sent. """ - logger.info( - "seer.explorer.slack.process_mention", - extra={ - "integration_id": integration_id, - "organization_id": organization_id, - "channel_id": channel_id, - "thread_ts": thread_ts, - "slack_user_id": slack_user_id, - }, + + with SlackEntrypointEventLifecycleMetric( + interaction_type=SlackEntrypointInteractionType.PROCESS_MENTION, + integration_id=integration_id, + organization_id=organization_id, + ).capture() as lifecycle: + lifecycle.add_extras( + { + "channel_id": channel_id, + "ts": ts, + "thread_ts": thread_ts, + "slack_user_id": slack_user_id, + }, + ) + + try: + organization = Organization.objects.get_from_cache(id=organization_id) + except Organization.DoesNotExist: + lifecycle.record_failure(failure_reason=ProcessMentionFailureReason.ORG_NOT_FOUND) + return + + if not SlackExplorerEntrypoint.has_access(organization): + lifecycle.record_failure(failure_reason=ProcessMentionFailureReason.NO_EXPLORER_ACCESS) + return + + try: + entrypoint = SlackExplorerEntrypoint( + integration_id=integration_id, + organization_id=organization_id, + channel_id=channel_id, + thread_ts=thread_ts or ts, + slack_user_id=slack_user_id, + ) + except (ValueError, EntrypointSetupError) as e: + lifecycle.record_failure(failure_reason=e) + return + + user = _resolve_user( + integration_external_id=entrypoint.integration.external_id, + slack_user_id=slack_user_id, + ) + if not user: + lifecycle.record_halt(ProcessMentionHaltReason.IDENTITY_NOT_LINKED) + # In a thread, show the prompt in the thread; top-level, show in the channel. + _send_link_identity_prompt( + entrypoint=entrypoint, + thread_ts=thread_ts if thread_ts else "", + ) + entrypoint.install.set_thread_status( + channel_id=channel_id, + thread_ts=thread_ts or ts, + status="", + ) + return + + if not organization.has_access(user): + lifecycle.record_halt(ProcessMentionHaltReason.USER_NOT_ORG_MEMBER) + _send_not_org_member_message( + entrypoint=entrypoint, + thread_ts=thread_ts if thread_ts else "", + org_name=organization.name, + ) + entrypoint.install.set_thread_status( + channel_id=channel_id, + thread_ts=thread_ts or ts, + status="", + ) + return + + prompt = extract_prompt(text, bot_user_id) + + # Only fetch thread context when actually in a thread. + thread_context: str | None = None + if thread_ts: + messages = entrypoint.install.get_thread_history( + channel_id=channel_id, thread_ts=thread_ts + ) + thread_context = build_thread_context(messages) or None + + operator = SeerExplorerOperator(entrypoint=entrypoint) + operator.trigger_explorer( + organization=organization, + user=user, + prompt=prompt, + on_page_context=thread_context, + category_key="slack_thread", + category_value=f"{channel_id}:{entrypoint.thread_ts}", + ) + + +def _resolve_user( + *, + integration_external_id: str, + slack_user_id: str, +) -> RpcUser | None: + """Resolve the Sentry user from a Slack user ID via linked identity.""" + provider = identity_service.get_provider( + provider_type=IntegrationProviderSlug.SLACK.value, + provider_ext_id=integration_external_id, + ) + if not provider: + return None + + identity = identity_service.get_identity( + filter={ + "provider_id": provider.id, + "identity_ext_id": slack_user_id, + } + ) + if not identity: + return None + + return user_service.get_user(identity.user_id) + + +def _send_link_identity_prompt( + *, + entrypoint: SlackExplorerEntrypoint, + thread_ts: str, +) -> None: + """Send an ephemeral message prompting the user to link their Slack identity to Sentry.""" + associate_url = build_linking_url( + integration=entrypoint.integration, + slack_id=entrypoint.slack_user_id, + channel_id=entrypoint.channel_id, + response_url=None, + ) + renderable = _build_link_identity_renderable(associate_url) + entrypoint.install.send_threaded_ephemeral_message( + slack_user_id=entrypoint.slack_user_id, + channel_id=entrypoint.channel_id, + renderable=renderable, + thread_ts=thread_ts, + ) + + +def _build_link_identity_renderable(associate_url: str) -> SlackRenderable: + """Build a SlackRenderable prompting the user to link their Slack identity to Sentry.""" + message = "Link your Slack identity to Sentry to use Seer Explorer in Slack." + return SlackRenderable( + blocks=[ + MarkdownBlock(text=message), + ActionsBlock( + elements=[ + LinkButtonElement(text="Link", url=associate_url), + ButtonElement(text="Cancel", value="ignore"), + ] + ), + ], + text=message, + ) + + +def _send_not_org_member_message( + *, + entrypoint: SlackExplorerEntrypoint, + thread_ts: str, + org_name: str, +) -> None: + """Send an ephemeral message informing the user they are not a member of the organization.""" + message = f"You must be a member of the *{org_name}* Sentry organization to use Seer Explorer in Slack." + renderable = SlackRenderable( + blocks=[MarkdownBlock(text=message)], + text=message, + ) + entrypoint.install.send_threaded_ephemeral_message( + slack_user_id=entrypoint.slack_user_id, + channel_id=entrypoint.channel_id, + renderable=renderable, + thread_ts=thread_ts, ) diff --git a/src/sentry/seer/similarity/similar_issues.py b/src/sentry/seer/similarity/similar_issues.py index 6e60d115f9654e..51dcae30f70958 100644 --- a/src/sentry/seer/similarity/similar_issues.py +++ b/src/sentry/seer/similarity/similar_issues.py @@ -76,8 +76,7 @@ def get_similarity_data_from_seer( logger_extra = { k: v for k, v in similar_issues_request.items() - if k - in {"event_id", "project_id", "hash", "referrer", "use_reranking", "model", "training_mode"} + if k in {"event_id", "project_id", "hash", "referrer", "model", "training_mode"} } logger.info( "get_seer_similar_issues.request", diff --git a/src/sentry/seer/similarity/types.py b/src/sentry/seer/similarity/types.py index 89030321d3b518..f6fd15ddfc61c0 100644 --- a/src/sentry/seer/similarity/types.py +++ b/src/sentry/seer/similarity/types.py @@ -38,7 +38,6 @@ class SimilarIssuesEmbeddingsRequest(TypedDict): read_only: NotRequired[bool] event_id: NotRequired[str] referrer: NotRequired[str] - use_reranking: NotRequired[bool] model: NotRequired[GroupingVersion] # Model version, defaults to V1 for backward compatibility training_mode: NotRequired[bool] # whether to just insert embedding without querying platform: NotRequired[str] diff --git a/src/sentry/sentry_apps/metrics.py b/src/sentry/sentry_apps/metrics.py index 4d0e060beb8bd2..72eb5fea395f7a 100644 --- a/src/sentry/sentry_apps/metrics.py +++ b/src/sentry/sentry_apps/metrics.py @@ -71,6 +71,7 @@ class SentryAppWebhookHaltReason(StrEnum): RESTRICTED_IP = "restricted_ip" CONNECTION_RESET = "connection_reset" HARD_TIMEOUT = "hard_timeout" + CIRCUIT_BROKEN = "circuit_broken" class SentryAppExternalRequestFailureReason(StrEnum): diff --git a/src/sentry/sentry_metrics/consumers/indexer/multiprocess.py b/src/sentry/sentry_metrics/consumers/indexer/multiprocess.py index 307fdfd50ed4f7..5b1042bd4664ae 100644 --- a/src/sentry/sentry_metrics/consumers/indexer/multiprocess.py +++ b/src/sentry/sentry_metrics/consumers/indexer/multiprocess.py @@ -93,7 +93,7 @@ def submit(self, message: Message[KafkaPayload | FilteredPayload]) -> None: on_delivery=partial( self.callback, committable=message.committable, timestamp=message.timestamp ), - headers=message.payload.headers, + headers=message.payload.headers, # type: ignore[arg-type] ) def callback( diff --git a/src/sentry/sentry_metrics/consumers/indexer/routing_producer.py b/src/sentry/sentry_metrics/consumers/indexer/routing_producer.py index 9690a923331213..57c2061cbec25f 100644 --- a/src/sentry/sentry_metrics/consumers/indexer/routing_producer.py +++ b/src/sentry/sentry_metrics/consumers/indexer/routing_producer.py @@ -118,7 +118,7 @@ def poll(self) -> None: def __delivery_callback( self, future: Future[str], - error: KafkaError, + error: KafkaError | None, message: ConfluentMessage, ) -> None: if error is not None: @@ -142,14 +142,12 @@ def submit(self, message: Message[RoutingPayload]) -> None: future: Future[str] = Future() future.set_running_or_notify_cancel() - ( - producer.produce( - topic=topic.name, - value=output_message.payload.value, - key=output_message.payload.key, - headers=output_message.payload.headers, - on_delivery=partial(self.__delivery_callback, future), - ), + producer.produce( + topic=topic.name, + value=output_message.payload.value, + key=output_message.payload.key, + headers=output_message.payload.headers, # type: ignore[arg-type] + on_delivery=partial(self.__delivery_callback, future), ) self.__queue.append((output_message.committable, future)) diff --git a/src/sentry/shared_integrations/exceptions/__init__.py b/src/sentry/shared_integrations/exceptions/__init__.py index b48955b19ec380..f65164182b6599 100644 --- a/src/sentry/shared_integrations/exceptions/__init__.py +++ b/src/sentry/shared_integrations/exceptions/__init__.py @@ -224,6 +224,6 @@ def __init__(self, field_errors: Mapping[str, Any] | None = None) -> None: class ClientError(RequestException): """4xx Error Occurred""" - def __init__(self, status_code: str, url: str, response: Response | None = None) -> None: + def __init__(self, status_code: str | int, url: str, response: Response | None = None) -> None: http_error_msg = f"{status_code} Client Error: for url: {url}" super().__init__(http_error_msg, response=response) diff --git a/src/sentry/snuba/referrer.py b/src/sentry/snuba/referrer.py index bc785e83708fcd..8996e68a9748db 100644 --- a/src/sentry/snuba/referrer.py +++ b/src/sentry/snuba/referrer.py @@ -836,6 +836,7 @@ class Referrer(StrEnum): ) TAGSTORE__GET_TAG_KEYS = "tagstore.__get_tag_keys" TAGSTORE_GET_GROUP_LIST_TAG_VALUE = "tagstore.get_group_list_tag_value" + TAGSTORE_GET_GROUP_TAG_VALUE_COUNT = "tagstore.get_group_tag_value_count" TAGSTORE_GET_GROUP_TAG_VALUE_ITER = "tagstore.get_group_tag_value_iter" TAGSTORE_GET_GROUPS_USER_COUNTS = "tagstore.get_groups_user_counts" TAGSTORE_GET_GROUPS_USER_COUNTS_GROUP_SNOOZE = "tagstore.get_groups_user_counts.groupsnooze" diff --git a/src/sentry/tagstore/snuba/backend.py b/src/sentry/tagstore/snuba/backend.py index e4946882019847..1df6578b8626cb 100644 --- a/src/sentry/tagstore/snuba/backend.py +++ b/src/sentry/tagstore/snuba/backend.py @@ -1,6 +1,7 @@ from __future__ import annotations import functools +import logging import os import re from collections import defaultdict @@ -30,6 +31,7 @@ from sentry.api.utils import default_start_end_dates, handle_query_errors from sentry.eventstream.item_helpers import format_attr_key from sentry.issues.grouptype import GroupCategory +from sentry.models.environment import Environment from sentry.models.group import Group from sentry.models.organization import Organization from sentry.models.project import Project @@ -38,6 +40,7 @@ from sentry.models.releaseprojectenvironment import ReleaseProjectEnvironment from sentry.models.releases.release_project import ReleaseProject from sentry.replays.query import query_replays_dataset_tagkey_values +from sentry.search.eap.occurrences.common_queries import count_occurrences from sentry.search.eap.occurrences.definitions import OCCURRENCE_DEFINITIONS from sentry.search.eap.occurrences.rollout_utils import EAPOccurrencesComparator from sentry.search.eap.resolver import SearchResolver @@ -56,6 +59,7 @@ from sentry.search.events.types import SnubaParams from sentry.services.eventstore.query_preprocessing import translate_environment_ids_to_names from sentry.snuba.dataset import Dataset +from sentry.snuba.occurrences_rpc import OccurrenceCategory, Occurrences from sentry.snuba.referrer import Referrer from sentry.tagstore.base import TOP_VALUES_DEFAULT_LIMIT, TagKeyStatus, TagStorage from sentry.tagstore.exceptions import GroupTagKeyNotFound, TagKeyNotFound @@ -70,6 +74,8 @@ raw_snql_query, ) +logger = logging.getLogger(__name__) + _max_unsampled_projects = 50 if os.environ.get("SENTRY_SINGLE_TENANT"): # This is a patch we used to have in single-tenant, but moving here @@ -133,6 +139,12 @@ def _translate_filter_keys( return forward(filter_keys) +def _reasonable_user_counts_match(control: dict[int, int], experimental: dict[int, int]) -> bool: + if not set(experimental.keys()).issubset(set(control.keys())): + return False + return all(experimental[group_id] <= control[group_id] for group_id in experimental) + + class _OptimizeKwargs(TypedDict, total=False): turbo: bool sample: int @@ -876,24 +888,64 @@ def apply_group_filters(self, group: Group | None, filters: MutableMapping[str, def get_group_tag_value_count( self, - group, - environment_id, + group: Group, + environment_id: int | None, key: str, - tenant_ids=None, - ): + tenant_ids: dict[str, str | int] | None = None, + ) -> int: filters: dict[str, Sequence[Any]] = {"project_id": get_project_list(group.project_id)} if environment_id: filters["environment"] = [environment_id] aggregations = [["count()", "", "count"]] dataset, filters = self.apply_group_filters(group, filters) - return snuba.query( + snuba_result = snuba.query( dataset=dataset, filter_keys=filters, aggregations=aggregations, - referrer="tagstore.get_group_tag_value_count", + referrer=Referrer.TAGSTORE_GET_GROUP_TAG_VALUE_COUNT.value, tenant_ids=tenant_ids, ) + result = snuba_result + + callsite = "SnubaTagStorage::get_group_tag_value_count" + if EAPOccurrencesComparator.should_check_experiment(callsite): + occurrence_category = ( + OccurrenceCategory.ERROR + if group.issue_category == GroupCategory.ERROR + else OccurrenceCategory.ISSUE_PLATFORM + ) + + now = datetime.now(tz=timezone.utc) + environments = ( + list(Environment.objects.filter(id=environment_id)) if environment_id else None + ) + + eap_result = count_occurrences( + organization=Organization.objects.get_from_cache(id=group.project.organization_id), + projects=[group.project], + start=now - timedelta(days=90), + end=now, + referrer=Referrer.TAGSTORE_GET_GROUP_TAG_VALUE_COUNT.value, + group_id=group.id, + environments=environments, + occurrence_category=occurrence_category, + ) + result = EAPOccurrencesComparator.check_and_choose( + control_data=snuba_result, + experimental_data=eap_result, + callsite=callsite, + is_experimental_data_a_null_result=eap_result == 0, + reasonable_match_comparator=lambda control, experimental: experimental <= control, + debug_context={ + "group_id": group.id, + "project_id": group.project_id, + "environment_id": environment_id, + "occurrence_category": occurrence_category.value, + }, + ) + + return result def get_top_group_tag_values( self, @@ -1166,7 +1218,7 @@ def get_groups_user_counts( tenant_ids: dict[str, str | int] | None = None, referrer: str = "tagstore.get_groups_user_counts", ) -> dict[int, int]: - return self.__get_groups_user_counts( + snuba_result = self.__get_groups_user_counts( project_ids, group_ids, environment_ids, @@ -1177,6 +1229,107 @@ def get_groups_user_counts( referrer, tenant_ids=tenant_ids, ) + result = snuba_result + + callsite = "SnubaTagStorage::get_groups_user_counts" + if EAPOccurrencesComparator.should_check_experiment(callsite): + eap_result = self._eap_get_groups_user_counts( + project_ids, + group_ids, + environment_ids, + start, + end, + referrer, + occurrence_category=OccurrenceCategory.ERROR, + ) + result = EAPOccurrencesComparator.check_and_choose( + control_data=snuba_result, + experimental_data=eap_result, + callsite=callsite, + is_experimental_data_a_null_result=len(eap_result) == 0, + reasonable_match_comparator=_reasonable_user_counts_match, + debug_context={ + "project_ids": list(project_ids), + "group_ids": list(group_ids), + "environment_ids": list(environment_ids) if environment_ids else None, + "start": start.isoformat() if start else None, + "end": end.isoformat() if end else None, + }, + ) + + return result + + def _eap_get_groups_user_counts( + self, + project_ids: Sequence[int], + group_ids: Sequence[int], + environment_ids: Sequence[int] | None, + start: datetime | None, + end: datetime | None, + referrer: str, + occurrence_category: OccurrenceCategory, + ) -> defaultdict[int, int]: + organization_id = get_organization_id_from_project_ids(project_ids) + + now = datetime.now(tz=timezone.utc) + resolved_start = start if start else now - timedelta(days=90) + resolved_end = end if end else now + + try: + organization = Organization.objects.get_from_cache(id=organization_id) + except Organization.DoesNotExist: + return defaultdict(int) + + projects = list(Project.objects.filter(id__in=project_ids)) + if not projects: + return defaultdict(int) + + environments = ( + list(Environment.objects.filter(id__in=environment_ids)) if environment_ids else [] + ) + + query_string = f"group_id:[{','.join(str(gid) for gid in group_ids)}]" + + snuba_params = SnubaParams( + start=resolved_start, + end=resolved_end, + organization=organization, + projects=projects, + environments=environments, + ) + + try: + result = Occurrences.run_table_query( + params=snuba_params, + query_string=query_string, + selected_columns=["group_id", "count_unique(user)"], + orderby=["-count_unique(user)"], + offset=0, + limit=len(group_ids), + referrer=referrer, + config=SearchResolverConfig(), + occurrence_category=occurrence_category, + ) + + return defaultdict( + int, + { + int(row["group_id"]): int(row["count_unique(user)"]) + for row in result.get("data", []) + if row.get("group_id") is not None and row.get("count_unique(user)") is not None + }, + ) + except Exception: + logger.exception( + "EAP groups user counts query failed", + extra={ + "organization_id": organization_id, + "project_ids": list(project_ids), + "group_ids": list(group_ids), + "occurrence_category": occurrence_category.value, + }, + ) + return defaultdict(int) def get_generic_groups_user_counts( self, @@ -1225,9 +1378,37 @@ def get_generic_groups_user_counts( result_snql = raw_snql_query(snuba_request, referrer=referrer, use_cache=True) - result = nest_groups(result_snql["data"], ["group_id"], ["count"]) + nested = nest_groups(result_snql["data"], ["group_id"], ["count"]) + snuba_result = defaultdict(int, {k: v for k, v in nested.items() if v}) + result = snuba_result + + callsite = "SnubaTagStorage::get_generic_groups_user_counts" + if EAPOccurrencesComparator.should_check_experiment(callsite): + eap_result = self._eap_get_groups_user_counts( + project_ids, + group_ids, + environment_ids, + start, + end, + referrer, + occurrence_category=OccurrenceCategory.ISSUE_PLATFORM, + ) + result = EAPOccurrencesComparator.check_and_choose( + control_data=snuba_result, + experimental_data=eap_result, + callsite=callsite, + is_experimental_data_a_null_result=len(eap_result) == 0, + reasonable_match_comparator=_reasonable_user_counts_match, + debug_context={ + "project_ids": list(project_ids), + "group_ids": list(group_ids), + "environment_ids": list(environment_ids) if environment_ids else None, + "start": start.isoformat() if start else None, + "end": end.isoformat() if end else None, + }, + ) - return defaultdict(int, {k: v for k, v in result.items() if v}) + return result def get_tag_value_paginator( self, diff --git a/src/sentry/taskworker/namespaces.py b/src/sentry/taskworker/namespaces.py index 2daca183a58eac..31502074bcae92 100644 --- a/src/sentry/taskworker/namespaces.py +++ b/src/sentry/taskworker/namespaces.py @@ -1,3 +1,5 @@ +from __future__ import annotations + from sentry.taskworker.runtime import app # Namespaces for taskworker tasks @@ -261,6 +263,9 @@ ) +# External namespaces for tasks belonging to other applications +launchpad_tasks = app.create_external_namespace(name="default", application="launchpad") + # Namespaces for testing taskworker tasks exampletasks = app.taskregistry.create_namespace(name="examples") test_tasks = app.taskregistry.create_namespace(name="test") diff --git a/src/sentry/testutils/factories.py b/src/sentry/testutils/factories.py index a3b1935bd8412d..aff0cddbce299b 100644 --- a/src/sentry/testutils/factories.py +++ b/src/sentry/testutils/factories.py @@ -378,22 +378,26 @@ def _set_sample_rate_from_error_sampling(normalized_data: MutableMapping[str, An class Factories: @staticmethod @assume_test_silo_mode(SiloMode.CELL) - def create_organization(name=None, owner=None, region: Cell | str | None = None, **kwargs): + def create_organization(name=None, owner=None, cell: Cell | str | None = None, **kwargs): + # TODO(cells): Remove once getsentry passes cell everywhere + if not cell: + cell = kwargs.pop("region", None) + if not name: name = petname.generate(2, " ", letters=10).title() with contextlib.ExitStack() as ctx: - if region is None or SiloMode.get_current_mode() == SiloMode.MONOLITH: - region_name = get_local_cell().name + if cell is None or SiloMode.get_current_mode() == SiloMode.MONOLITH: + cell_name = get_local_cell().name else: - if isinstance(region, Cell): - region_name = region.name + if isinstance(cell, Cell): + cell_name = cell.name else: - region_obj = get_cell_by_name(region) # Verify it exists - region_name = region_obj.name + cell_obj = get_cell_by_name(cell) # Verify it exists + cell_name = cell_obj.name ctx.enter_context( - override_settings(SILO_MODE=SiloMode.CELL, SENTRY_REGION=region_name) + override_settings(SILO_MODE=SiloMode.CELL, SENTRY_REGION=cell_name) ) with outbox_context(flush=False): @@ -403,7 +407,7 @@ def create_organization(name=None, owner=None, region: Cell | str | None = None, # Organization mapping creation relies on having a matching org slug reservation OrganizationSlugReservation( organization_id=org.id, - cell_name=region_name, + cell_name=cell_name, user_id=owner.id if owner else -1, slug=org.slug, ).save(unsafe_write=True) @@ -430,7 +434,7 @@ def create_org_mapping(org=None, **kwds): kwds.setdefault("slug", org.slug) kwds.setdefault("name", org.name) kwds.setdefault("idempotency_key", uuid4().hex) - kwds.setdefault("region_name", "na") + kwds.setdefault("cell_name", "na") return OrganizationMapping.objects.create(**kwds) @staticmethod diff --git a/src/sentry/testutils/helpers/apigateway.py b/src/sentry/testutils/helpers/apigateway.py index bc3cde3f74f5cf..1c78bdd2c16325 100644 --- a/src/sentry/testutils/helpers/apigateway.py +++ b/src/sentry/testutils/helpers/apigateway.py @@ -255,7 +255,7 @@ def setUp(self): headers={"test": "header"}, ) - self.organization = self.create_organization(region=self.CELL) + self.organization = self.create_organization(cell=self.CELL) # Echos the request body and header back for verification def return_request_body(request: httpx.Request): diff --git a/src/sentry/testutils/outbox.py b/src/sentry/testutils/outbox.py index 98da1a967b66a5..7f38bb13d2c830 100644 --- a/src/sentry/testutils/outbox.py +++ b/src/sentry/testutils/outbox.py @@ -64,7 +64,10 @@ def assert_no_webhook_payloads() -> None: def assert_webhook_payloads_for_mailbox( request: WSGIRequest, mailbox_name: str, - region_names: list[str], + # TODO(cells): make required once getsentry passes cell everywhere + cell_names: list[str] | None = None, + # TODO(cells): remove once getsentry passes cell everywhere + region_names: list[str] | None = None, destination_types: dict[DestinationType, int] | None = None, ) -> None: """ @@ -73,16 +76,16 @@ def assert_webhook_payloads_for_mailbox( :param request: :param mailbox_name: The mailbox name that messages should be found in. - :param region_names: Optional list of regions each messages should be queued for + :param cell_names: List of cells each messages should be queued for :param destination_types: Optional Mapping of destination types to the number of messages that should be found for that destination type """ expected_payload = WebhookPayload.get_attributes_from_request(request=request) - region_names_set = set(region_names) + cell_names_set = set(cell_names or region_names or []) messages = WebhookPayload.objects.filter(mailbox_name=mailbox_name) - messages_with_region_count = messages.filter(cell_name__isnull=False).count() - if messages_with_region_count != len(region_names_set): + messages_with_cell_count = messages.filter(cell_name__isnull=False).count() + if messages_with_cell_count != len(cell_names_set): raise Exception( - f"Mismatch: Found {messages_with_region_count} WebhookPayload but {len(region_names_set)} region_names" + f"Mismatch: Found {messages_with_cell_count} WebhookPayload but {len(cell_names_set)} cell_names" ) for message in messages: assert message.request_method == expected_payload["request_method"] @@ -104,13 +107,13 @@ def assert_webhook_payloads_for_mailbox( else: assert message.cell_name is not None try: - region_names_set.remove(message.cell_name) + cell_names_set.remove(message.cell_name) except KeyError: raise Exception( - f"Found ControlOutbox for '{message.cell_name}', which was not in region_names: {str(region_names_set)}" + f"Found ControlOutbox for '{message.cell_name}', which was not in cell_names: {str(cell_names_set)}" ) - if len(region_names_set) != 0: - raise Exception(f"WebhookPayload not found for some region_names: {str(region_names_set)}") + if len(cell_names_set) != 0: + raise Exception(f"WebhookPayload not found for some cell_names: {str(cell_names_set)}") if destination_types and len(destination_types) != 0: exc_strs = [ diff --git a/src/sentry/testutils/pytest/kafka.py b/src/sentry/testutils/pytest/kafka.py index 80475030cc8794..f0cd95a6179d31 100644 --- a/src/sentry/testutils/pytest/kafka.py +++ b/src/sentry/testutils/pytest/kafka.py @@ -4,7 +4,8 @@ from collections.abc import MutableMapping import pytest -from confluent_kafka import Consumer, Producer +from arroyo.processing import StreamProcessor +from confluent_kafka import Producer from confluent_kafka.admin import AdminClient from sentry.testutils.pytest import xdist @@ -73,7 +74,7 @@ def scope_consumers(): be created once per test session). """ - all_consumers: MutableMapping[str, Consumer | None] = { + all_consumers: MutableMapping[str, StreamProcessor | None] = { xdist.get_kafka_topic("ingest-events"): None, xdist.get_kafka_topic("outcomes"): None, } diff --git a/src/sentry/utils/batching_kafka_consumer.py b/src/sentry/utils/batching_kafka_consumer.py index 47f0530e32b42c..9442975cdb2b29 100644 --- a/src/sentry/utils/batching_kafka_consumer.py +++ b/src/sentry/utils/batching_kafka_consumer.py @@ -15,7 +15,7 @@ def wait_for_topics(admin_client: AdminClient, topics: list[str], timeout: int = """ for topic in topics: start = time.time() - last_error = None + last_error: str | KafkaError | None = None while True: if time.time() > start + timeout: @@ -25,10 +25,14 @@ def wait_for_topics(admin_client: AdminClient, topics: list[str], timeout: int = result = admin_client.list_topics(topic=topic, timeout=timeout) topic_metadata = result.topics.get(topic) - if topic_metadata and topic_metadata.partitions and not topic_metadata.error: + if topic_metadata is None: + last_error = "Topic metadata not found" + time.sleep(0.1) + continue + if topic_metadata.partitions and not topic_metadata.error: logger.debug("Topic '%s' is ready", topic) break - elif topic_metadata.error in { + elif topic_metadata.error is not None and topic_metadata.error.code() in { KafkaError.UNKNOWN_TOPIC_OR_PART, KafkaError.LEADER_NOT_AVAILABLE, }: diff --git a/src/sentry/utils/jwt.py b/src/sentry/utils/jwt.py index 462e1dee48a38a..f35585193140c7 100644 --- a/src/sentry/utils/jwt.py +++ b/src/sentry/utils/jwt.py @@ -18,6 +18,7 @@ PublicFormat, ) from jwt import DecodeError +from jwt.types import Options __all__ = ["peek_claims", "decode", "encode", "authorization_header", "DecodeError"] @@ -65,7 +66,7 @@ def decode( """ # TODO: We do not currently have type-safety for keys suitable for decoding *and* # encoding vs those only suitable for decoding. - options = {} + options: Options = {} kwargs: dict[str, Any] = {} if audience is False: options["verify_aud"] = False diff --git a/src/sentry/utils/sentry_apps/circuit_breaker.py b/src/sentry/utils/sentry_apps/circuit_breaker.py new file mode 100644 index 00000000000000..34dbd8591a475a --- /dev/null +++ b/src/sentry/utils/sentry_apps/circuit_breaker.py @@ -0,0 +1,39 @@ +import logging +from collections.abc import Generator +from contextlib import contextmanager + +from sentry.utils.circuit_breaker2 import CircuitBreaker + +logger = logging.getLogger("sentry.sentry_apps.circuit_breaker") + + +@contextmanager +def circuit_breaker_tracking( + breaker: CircuitBreaker | None, +) -> Generator[None]: + """Track request outcome: record_error on WebhookTimeoutError, record_success on normal exit. + + Handles the None case as a no-op so callers don't need nullcontext(). + """ + from sentry.utils.sentry_apps.webhooks import WebhookTimeoutError + + if breaker is None: + yield + return + try: + yield + + # Currently we only count WebhookTimeoutError as an error in the circuit breaker as those operations are the ones that are taking too long + # If an app returns a say 500, in a reasonable time that's okay + except WebhookTimeoutError: + # This is gross but we don't want to propagate a redis or circuit breaker error to the webhook code + try: + breaker.record_error() + except Exception: + logger.exception("sentry_apps.circuit_breaker.record_error.failure") + raise + else: + try: + breaker.record_success() + except Exception: + logger.exception("sentry_apps.circuit_breaker.record_success.failure") diff --git a/src/sentry/utils/sentry_apps/webhooks.py b/src/sentry/utils/sentry_apps/webhooks.py index 7e84de7d41e658..7c5380f75f4206 100644 --- a/src/sentry/utils/sentry_apps/webhooks.py +++ b/src/sentry/utils/sentry_apps/webhooks.py @@ -4,6 +4,7 @@ from collections.abc import Callable, Mapping from types import FrameType from typing import TYPE_CHECKING, Any, Concatenate, ParamSpec, TypeVar +from urllib.parse import urlparse import sentry_sdk from requests import RequestException, Response @@ -13,6 +14,8 @@ from sentry import features, options from sentry.exceptions import RestrictedIPAddress from sentry.http import safe_urlopen +from sentry.integrations.utils.metrics import EventLifecycle +from sentry.organizations.services.organization.model import RpcUserOrganizationContext from sentry.organizations.services.organization.service import organization_service from sentry.sentry_apps.metrics import ( SentryAppEventType, @@ -23,7 +26,10 @@ from sentry.sentry_apps.utils.errors import SentryAppSentryError from sentry.shared_integrations.exceptions import ApiHostError, ApiTimeoutError, ClientError from sentry.taskworker.timeout import timeout_alarm +from sentry.utils import metrics +from sentry.utils.circuit_breaker2 import CircuitBreaker, RateBasedTripStrategy from sentry.utils.sentry_apps import SentryAppWebhookRequestsBuffer +from sentry.utils.sentry_apps.circuit_breaker import circuit_breaker_tracking if TYPE_CHECKING: from sentry.sentry_apps.api.serializers.app_platform_event import AppPlatformEvent @@ -51,7 +57,6 @@ def _handle_webhook_timeout(signum: int, frame: FrameType | None) -> None: """Handler for when a webhook request exceeds the hard timeout deadline. - This is a workaround for safe_create_connection sockets hanging when the given url cannot be reached or resolved. - - TODO(christinarlong): Add sentry app disabling logic here """ raise WebhookTimeoutError("Webhook request exceeded hard timeout deadline") @@ -73,6 +78,79 @@ def wrapper( return wrapper +def _create_circuit_breaker( + sentry_app: SentryApp | RpcSentryApp, + organization_context: RpcUserOrganizationContext | None, +) -> CircuitBreaker | None: + if organization_context is None or not features.has( + "organizations:sentry-app-webhook-circuit-breaker", + organization_context.organization, + ): + return None + config = options.get("sentry-apps.webhook.circuit-breaker.config") + return CircuitBreaker( + key=f"sentry-app.webhook.{sentry_app.slug}", + config=config, + trip_strategy=RateBasedTripStrategy.from_config(config), + ) + + +def _circuit_breaker_allows_request( + circuit_breaker: CircuitBreaker | None, + sentry_app: SentryApp | RpcSentryApp, + org_id: int, + lifecycle: EventLifecycle, +) -> bool: + if circuit_breaker is None or circuit_breaker.should_allow_request(): + return True + + dry_run = options.get("sentry-apps.webhook.circuit-breaker.dry-run") + if dry_run: + metrics.incr( + "sentry_app.webhook.circuit_breaker.would_block", + tags={"slug": sentry_app.slug}, + ) + logger.warning( + "sentry_app.webhook.circuit_breaker.would_block", + extra={"slug": sentry_app.slug, "org_id": org_id}, + ) + return True + + lifecycle.record_halt( + halt_reason=f"send_and_save_webhook_request.{SentryAppWebhookHaltReason.CIRCUIT_BROKEN}" + ) + return False + + +def _send_webhook_request( + url: str, + app_platform_event: AppPlatformEvent[T], + organization_context: RpcUserOrganizationContext | None, +) -> Response: + if organization_context is not None and features.has( + "organizations:sentry-app-webhook-hard-timeout", + organization_context.organization, + ): + # We're using a signal based timeout here because we need to interrupt the blocking + # socket.connect() operation. See SENTRY-5HA6 for more context. Here we're hanging at + # the socket.connect() call and the timeout we set in safe_urlopen is not being respected. + timeout_seconds = options.get("sentry-apps.webhook.hard-timeout.sec") + with timeout_alarm(timeout_seconds, _handle_webhook_timeout): + return safe_urlopen( + url=url, + data=app_platform_event.body, + headers=app_platform_event.headers, + timeout=options.get("sentry-apps.webhook.timeout.sec"), + ) + + return safe_urlopen( + url=url, + data=app_platform_event.body, + headers=app_platform_event.headers, + timeout=options.get("sentry-apps.webhook.timeout.sec"), + ) + + @sentry_sdk.trace(name="send_and_save_webhook_request") @ignore_unpublished_app_errors def send_and_save_webhook_request( @@ -124,28 +202,12 @@ def send_and_save_webhook_request( include_projects=False, include_teams=False, ) - if organization_context is not None and features.has( - "organizations:sentry-app-webhook-hard-timeout", - organization_context.organization, - ): - # We're using a signal based timeout here because we need to interrupt the blocking socket.connect() opeartion. - # See SENTRY-5HA6 for more context. Here we're hanging at the socket.connect() call and the timeout we set - # in safe_urlopen is not being respected. - timeout_seconds = options.get("sentry-apps.webhook.hard-timeout.sec") - with timeout_alarm(timeout_seconds, _handle_webhook_timeout): - response = safe_urlopen( - url=url, - data=app_platform_event.body, - headers=app_platform_event.headers, - timeout=options.get("sentry-apps.webhook.timeout.sec"), - ) - else: - response = safe_urlopen( - url=url, - data=app_platform_event.body, - headers=app_platform_event.headers, - timeout=options.get("sentry-apps.webhook.timeout.sec"), - ) + circuit_breaker = _create_circuit_breaker(sentry_app, organization_context) + if not _circuit_breaker_allows_request(circuit_breaker, sentry_app, org_id, lifecycle): + return Response() + + with circuit_breaker_tracking(circuit_breaker): + response = _send_webhook_request(url, app_platform_event, organization_context) except WebhookTimeoutError: lifecycle.record_halt( @@ -186,13 +248,19 @@ def send_and_save_webhook_request( raise track_response_code(response.status_code, slug, event) + + project_id = ( + int(p_id) + if (p_id := response.headers.get("Sentry-Hook-Project")) and p_id.isdigit() + else None + ) buffer.add_request( response_code=response.status_code, org_id=org_id, event=event, url=url, error_id=response.headers.get("Sentry-Hook-Error"), - project_id=response.headers.get("Sentry-Hook-Project"), + project_id=project_id, response=response, headers=app_platform_event.headers, ) @@ -223,13 +291,15 @@ def send_and_save_webhook_request( lifecycle.record_halt( halt_reason=f"send_and_save_webhook_request.{SentryAppWebhookHaltReason.INTEGRATOR_ERROR}" ) - raise ApiHostError.from_request(response.request) + raise ApiHostError(f"Unable to reach host: {urlparse(url).netloc}", url=url) elif response.status_code == status.HTTP_504_GATEWAY_TIMEOUT: lifecycle.record_halt( halt_reason=f"send_and_save_webhook_request.{SentryAppWebhookHaltReason.INTEGRATOR_ERROR}" ) - raise ApiTimeoutError.from_request(response.request) + raise ApiTimeoutError( + f"Timed out attempting to reach host: {urlparse(url).netloc}", url=url + ) elif 400 <= response.status_code < 500: lifecycle.record_halt( @@ -243,4 +313,5 @@ def send_and_save_webhook_request( except RequestException as e: lifecycle.record_halt(e) raise + return response diff --git a/src/sentry/utils/snuba.py b/src/sentry/utils/snuba.py index 219b7eff37e292..8021ed233921fd 100644 --- a/src/sentry/utils/snuba.py +++ b/src/sentry/utils/snuba.py @@ -974,7 +974,13 @@ def _preprocess_group_id_redirects(self): # just subtract the NOT IN groups from the IN groups. if in_groups is not None: in_groups.difference_update(out_groups) - triple = ["group_id", "IN", get_all_merged_group_ids(in_groups)] + + # An "group_id IN ()" clause breaks clickhouse. + # Better to make the exception (& expectations) clear. + if len(in_groups) > 0: + triple = ["group_id", "IN", get_all_merged_group_ids(in_groups)] + else: + raise SnubaError("Found empty intersection of group_ids") elif len(out_groups) > 0: triple = ["group_id", "NOT IN", out_groups] diff --git a/src/sentry/web/frontend/pipeline_advancer.py b/src/sentry/web/frontend/pipeline_advancer.py index 08113e4c3f45c4..ffbdef10bebe64 100644 --- a/src/sentry/web/frontend/pipeline_advancer.py +++ b/src/sentry/web/frontend/pipeline_advancer.py @@ -1,14 +1,18 @@ +from urllib.parse import parse_qs + from django.contrib import messages -from django.http import HttpRequest, HttpResponseRedirect +from django.http import HttpRequest, HttpResponse, HttpResponseRedirect from django.http.response import HttpResponseBase from django.urls import reverse from django.utils.translation import gettext_lazy as _ +from sentry import features from sentry.identity.pipeline import IdentityPipeline from sentry.integrations.pipeline import IntegrationPipeline from sentry.integrations.types import IntegrationProviderSlug from sentry.organizations.absolute_url import generate_organization_url from sentry.utils.http import absolute_uri, create_redirect_url +from sentry.utils.json import dumps_htmlsafe from sentry.web.frontend.base import BaseView, all_silo_view # The request doesn't contain the pipeline type (pipeline information is stored @@ -16,10 +20,70 @@ # and use whichever one works. PIPELINE_CLASSES = (IntegrationPipeline, IdentityPipeline) +TRAMPOLINE_HTML = """\ + + + + + + + +""" + + +def _render_trampoline(request: HttpRequest, pipeline: object) -> HttpResponse: + """Render a minimal page that posts callback params back to the opener.""" + params: dict[str, str] = {"source": "sentry-pipeline"} + for key, values in parse_qs(request.META.get("QUERY_STRING", "")).items(): + if values: + params[key] = values[0] + + data_json = str(dumps_htmlsafe(params)) + + # In multi-region the opener may be on a different origin (e.g. + # org-slug.sentry.io) than the trampoline (sentry.io/extensions/...), + # so we need the org-specific URL. In single-region document.origin works. + if features.has("system:multi-region"): + org = getattr(pipeline, "organization", None) + origin = str(dumps_htmlsafe(generate_organization_url(org.slug if org else ""))) + else: + origin = "document.origin" + + return HttpResponse( + TRAMPOLINE_HTML.format(data_json=data_json, origin=origin), + content_type="text/html", + ) + @all_silo_view class PipelineAdvancerView(BaseView): - """Gets the current pipeline from the request and executes the current step.""" + """ + Gets the current pipeline from the request and executes the current step. + + External services (e.g. GitHub OAuth) redirect back to this view after the + user completes an action. For legacy template-driven pipelines this view + processes the callback server-side via pipeline.current_step(). + + For API-driven pipelines (is_api_mode) this view does NOT process the + callback. Instead it renders a lightweight trampoline page that relays the + callback URL query params (code, state, installation_id, etc.) back to the + opener window via postMessage and closes itself. The frontend is + responsible for POSTing those params to the pipeline API endpoint to + advance the pipeline. + """ auth_required = False @@ -50,6 +114,12 @@ def handle(self, request: HttpRequest, provider_id: str) -> HttpResponseBase: messages.add_message(request, messages.ERROR, _("Invalid request.")) return self.redirect("/") + # If the pipeline was initiated via the API, render a trampoline page + # that relays the callback params back to the opener window via + # postMessage instead of processing the callback server-side. + if pipeline.is_api_mode: + return _render_trampoline(request, pipeline) + subdomain = pipeline.fetch_state("subdomain") if subdomain is not None and request.subdomain != subdomain: url_prefix = generate_organization_url(subdomain) diff --git a/src/sentry_plugins/github/client.py b/src/sentry_plugins/github/client.py index e12576f27db10f..6bcc067ec069d4 100644 --- a/src/sentry_plugins/github/client.py +++ b/src/sentry_plugins/github/client.py @@ -99,7 +99,7 @@ def get_jwt(self) -> str: # JWT expiration time (10 minute maximum) "exp": exp, # Integration's GitHub identifier - "iss": options.get("github.integration-app-id"), + "iss": str(options.get("github.integration-app-id")), } return jwt.encode(payload, options.get("github.integration-private-key"), algorithm="RS256") diff --git a/static/app/components/core/compactSelect/composite.mdx b/static/app/components/core/compactSelect/composite.mdx index 55f9b2a1126477..94c4a9045cdb45 100644 --- a/static/app/components/core/compactSelect/composite.mdx +++ b/static/app/components/core/compactSelect/composite.mdx @@ -53,6 +53,7 @@ Use `` when you need a dropdown with multiple independent selec - **``**: The wrapper component that manages the dropdown - **``**: Individual selection sections within the dropdown +- **``**: A "Clear" button for use in `menuHeaderTrailingItems` that calls your `onClick` to reset all regions. Re-exported from ``'s internal clear button for visual consistency Each region acts like an independent ``, requiring its own `value`, `onChange`, and `options`. @@ -124,6 +125,102 @@ const [day, setDay] = useState('1'); ; ``` +## Clear Button + +Use `` in `menuHeaderTrailingItems` to add a "Clear" button to the menu header. When clicked, it calls your `onClick` handler where you reset each region's value. The menu stays open, matching the behavior of the built-in clear button in ``. The component is the same styled button used internally by ``, ensuring visual consistency. + +Only render the button when there is an active selection to clear. + +export function ClearButtonDemo() { + const monthOptions = [ + {value: 'jan', label: 'January'}, + {value: 'feb', label: 'February'}, + {value: 'mar', label: 'March'}, + ]; + const tagOptions = [ + {value: 'cool', label: 'cool'}, + {value: 'funny', label: 'funny'}, + {value: 'awesome', label: 'awesome'}, + ]; + const [month, setMonth] = useState(''); + const [tags, setTags] = useState(tagOptions.slice(0, 0).map(o => o.value)); + const hasSelection = month !== '' || tags.length > 0; + return ( + { + setMonth(''); + setTags([]); + }} + /> + ) : null + } + trigger={props => ( + } {...props}> + Filters + + )} + > + setMonth(selection.value)} + options={monthOptions} + /> + setTags(selection.map(s => s.value))} + options={tagOptions} + /> + + ); +} + + + + + +```jsx +const [month, setMonth] = useState(null); +const [tags, setTags] = useState([]); +const hasSelection = month !== null || tags.length > 0; + + { + setMonth(null); + setTags([]); + }} + /> + ) : null + } + trigger={props => Filters} +> + setMonth(selection.value)} + options={monthOptions} + /> + setTags(selection.map(s => s.value))} + options={tagOptions} + /> +; +``` + ## Multi-Select Regions Individual regions can enable multi-select by setting the `multiple` prop. This allows mixing single and multi-select behavior within the same dropdown. diff --git a/static/app/components/core/compactSelect/composite.tsx b/static/app/components/core/compactSelect/composite.tsx index 7d67012bf994f7..34ea6b5ce98abf 100644 --- a/static/app/components/core/compactSelect/composite.tsx +++ b/static/app/components/core/compactSelect/composite.tsx @@ -4,10 +4,12 @@ import {FocusScope} from '@react-aria/focus'; import {Item} from '@react-stately/collections'; import type {DistributedOmit} from 'type-fest'; +import {type ButtonProps} from '@sentry/scraps/button'; + import {t} from 'sentry/locale'; +import {ClearButton, Control} from './control'; import type {ControlProps} from './control'; -import {Control} from './control'; import type {MultipleListProps, SingleListProps} from './list'; import {List} from './list'; import {EmptyMessage} from './styles'; @@ -118,6 +120,16 @@ CompositeSelect.Region = function ( return null; }; +CompositeSelect.ClearButton = function CompositeSelectClearButton( + props: DistributedOmit +) { + return ( + + {t('Clear')} + + ); +}; + export {CompositeSelect}; type RegionProps = CompositeSelectRegion & { diff --git a/static/app/components/core/compactSelect/control.tsx b/static/app/components/core/compactSelect/control.tsx index 493efbef4a61c2..bc99eecc9b20f2 100644 --- a/static/app/components/core/compactSelect/control.tsx +++ b/static/app/components/core/compactSelect/control.tsx @@ -643,7 +643,7 @@ const StyledLoadingIndicator = styled(LoadingIndicator)` } `; -const ClearButton = styled(Button)` +export const ClearButton = styled(Button)` font-size: inherit; /* Inherit font size from MenuHeader */ font-weight: ${p => p.theme.font.weight.sans.regular}; color: ${p => p.theme.tokens.content.secondary}; diff --git a/static/app/components/events/autofix/v3/autofixCards.tsx b/static/app/components/events/autofix/v3/autofixCards.tsx index fa4b42347aa0bc..a13d58f4022246 100644 --- a/static/app/components/events/autofix/v3/autofixCards.tsx +++ b/static/app/components/events/autofix/v3/autofixCards.tsx @@ -370,7 +370,7 @@ interface ArtifactCardProps { function ArtifactCard({children, icon, title}: ArtifactCardProps) { return ( - + @@ -379,7 +379,7 @@ function ArtifactCard({children, icon, title}: ArtifactCardProps) { - + {children} @@ -394,7 +394,7 @@ interface ArtifactDetailsProps extends FlexProps { function ArtifactDetails({children, ...flexProps}: ArtifactDetailsProps) { return ( - + {children} ); diff --git a/static/app/components/feedback/decodeFeedbackSlug.tsx b/static/app/components/feedback/decodeFeedbackSlug.tsx deleted file mode 100644 index 2f2d9b973e32eb..00000000000000 --- a/static/app/components/feedback/decodeFeedbackSlug.tsx +++ /dev/null @@ -1,13 +0,0 @@ -import {decodeScalar} from 'sentry/utils/queryString'; - -// TypeScript thinks the values are strings, but they could be undefined. -// See `noUncheckedIndexedAccess` -interface Return { - feedbackId: string | undefined; - projectSlug: string | undefined; -} - -export function decodeFeedbackSlug(val: string | string[] | null | undefined): Return { - const [projectSlug, feedbackId] = decodeScalar(val, '').split(':'); - return {projectSlug, feedbackId}; -} diff --git a/static/app/components/feedback/feedbackItem/feedbackItemLoader.tsx b/static/app/components/feedback/feedbackItem/feedbackItemLoader.tsx index 07ae5a5486978c..f55033feecfaae 100644 --- a/static/app/components/feedback/feedbackItem/feedbackItemLoader.tsx +++ b/static/app/components/feedback/feedbackItem/feedbackItemLoader.tsx @@ -4,8 +4,7 @@ import {ErrorBoundary} from 'sentry/components/errorBoundary'; import {FeedbackEmptyDetails} from 'sentry/components/feedback/details/feedbackEmptyDetails'; import {FeedbackErrorDetails} from 'sentry/components/feedback/details/feedbackErrorDetails'; import {FeedbackItem} from 'sentry/components/feedback/feedbackItem/feedbackItem'; -import {useCurrentFeedbackId} from 'sentry/components/feedback/useCurrentFeedbackId'; -import {useCurrentFeedbackProject} from 'sentry/components/feedback/useCurrentFeedbackProject'; +import {useFeedbackSlug} from 'sentry/components/feedback/useFeedbackSlug'; import {useFetchFeedbackData} from 'sentry/components/feedback/useFetchFeedbackData'; import {Placeholder} from 'sentry/components/placeholder'; import {t} from 'sentry/locale'; @@ -19,11 +18,12 @@ interface Props { export function FeedbackItemLoader({onBackToList}: Props = {}) { const organization = useOrganization(); - const feedbackId = useCurrentFeedbackId(); - const {issueResult, issueData, eventData} = useFetchFeedbackData({feedbackId}); + const [feedbackSlug] = useFeedbackSlug(); + const feedbackId = feedbackSlug?.feedbackId ?? ''; - const projectSlug = useCurrentFeedbackProject(); - useSentryAppComponentsData({projectId: projectSlug}); + const {issueResult, issueData, eventData} = useFetchFeedbackData({feedbackId}); + const projectId = issueData?.project?.id ?? feedbackSlug?.projectSlug; + useSentryAppComponentsData({projectId}); useEffect(() => { if (issueResult.isError) { diff --git a/static/app/components/feedback/feedbackItem/feedbackShortId.tsx b/static/app/components/feedback/feedbackItem/feedbackShortId.tsx index ec7eef5e1495ba..012a14a25bc7dc 100644 --- a/static/app/components/feedback/feedbackItem/feedbackShortId.tsx +++ b/static/app/components/feedback/feedbackItem/feedbackShortId.tsx @@ -6,7 +6,6 @@ import queryString from 'query-string'; import {Flex} from '@sentry/scraps/layout'; import {DropdownMenu} from 'sentry/components/dropdownMenu'; -import {useCurrentFeedbackProject} from 'sentry/components/feedback/useCurrentFeedbackProject'; import ProjectBadge from 'sentry/components/idBadge/projectBadge'; import {TextOverflow} from 'sentry/components/textOverflow'; import {IconChevron} from 'sentry/icons'; @@ -38,7 +37,7 @@ const hideDropdown = css` export function FeedbackShortId({className, feedbackItem, style}: Props) { const organization = useOrganization(); - const projectSlug = useCurrentFeedbackProject(); + const projectSlug = feedbackItem.project?.slug ?? ''; // we need the stringifyUrl part so that the whole item is a string // for the copy url button below. normalizeUrl can return an object if `query` @@ -52,7 +51,7 @@ export function FeedbackShortId({className, feedbackItem, style}: Props) { queryString.stringifyUrl({ url: '?', query: { - feedbackSlug: `${projectSlug}:${feedbackItem.id}`, + feedbackSlug: projectSlug ? `${projectSlug}:${feedbackItem.id}` : feedbackItem.id, project: feedbackItem.project?.id, }, }); diff --git a/static/app/components/feedback/getFeedbackItemQueryKey.tsx b/static/app/components/feedback/getFeedbackItemQueryKey.tsx index 6c343a3212140f..bd33c083436f1e 100644 --- a/static/app/components/feedback/getFeedbackItemQueryKey.tsx +++ b/static/app/components/feedback/getFeedbackItemQueryKey.tsx @@ -19,7 +19,7 @@ export function getFeedbackItemQueryKey({feedbackId, organization}: Props): { }), { query: { - collapse: ['release', 'tags'], + collapse: ['release', 'tags', 'stats'], }, }, ] @@ -36,6 +36,11 @@ export function getFeedbackItemQueryKey({feedbackId, organization}: Props): { }, } ), + { + query: { + collapse: ['fullRelease'], + }, + }, ] : undefined, }; diff --git a/static/app/components/feedback/useCurrentFeedbackId.tsx b/static/app/components/feedback/useCurrentFeedbackId.tsx deleted file mode 100644 index c98e011abab95d..00000000000000 --- a/static/app/components/feedback/useCurrentFeedbackId.tsx +++ /dev/null @@ -1,12 +0,0 @@ -import {decodeFeedbackSlug} from 'sentry/components/feedback/decodeFeedbackSlug'; -import {useLocationQuery} from 'sentry/utils/url/useLocationQuery'; - -// See also: useCurrentFeedbackProject() - -export function useCurrentFeedbackId() { - const {feedbackSlug: feedbackId} = useLocationQuery({ - fields: {feedbackSlug: (val: any) => decodeFeedbackSlug(val).feedbackId ?? ''}, - }); - - return feedbackId; -} diff --git a/static/app/components/feedback/useCurrentFeedbackProject.tsx b/static/app/components/feedback/useCurrentFeedbackProject.tsx deleted file mode 100644 index 3d9113ec35aeac..00000000000000 --- a/static/app/components/feedback/useCurrentFeedbackProject.tsx +++ /dev/null @@ -1,12 +0,0 @@ -import {decodeFeedbackSlug} from 'sentry/components/feedback/decodeFeedbackSlug'; -import {useLocationQuery} from 'sentry/utils/url/useLocationQuery'; - -// See also: useCurrentFeedbackId() - -export function useCurrentFeedbackProject() { - const {feedbackSlug: projectSlug} = useLocationQuery({ - fields: {feedbackSlug: (val: any) => decodeFeedbackSlug(val).projectSlug ?? ''}, - }); - - return projectSlug; -} diff --git a/static/app/components/feedback/useFeedbackSlug.tsx b/static/app/components/feedback/useFeedbackSlug.tsx new file mode 100644 index 00000000000000..e339a6c144eb78 --- /dev/null +++ b/static/app/components/feedback/useFeedbackSlug.tsx @@ -0,0 +1,24 @@ +import {createParser, useQueryState} from 'nuqs'; + +type FeedbackSlug = { + feedbackId: string; + projectSlug: string; +}; + +const parseFeedbackSlug = createParser({ + parse: value => { + if (value.includes(':')) { + const [projectSlug, feedbackId] = value.split(':'); + return {projectSlug: projectSlug!, feedbackId: feedbackId!}; + } + return {feedbackId: value, projectSlug: ''}; + }, + serialize: value => + value + ? value.projectSlug + ? `${value.projectSlug}:${value.feedbackId}` + : value.feedbackId + : '', +}); + +export const useFeedbackSlug = () => useQueryState('feedbackSlug', parseFeedbackSlug); diff --git a/static/app/components/feedback/useRedirectToFeedbackFromEvent.tsx b/static/app/components/feedback/useRedirectToFeedbackFromEvent.tsx index 4c71e02c2ffe69..24ac7328cb374e 100644 --- a/static/app/components/feedback/useRedirectToFeedbackFromEvent.tsx +++ b/static/app/components/feedback/useRedirectToFeedbackFromEvent.tsx @@ -1,10 +1,9 @@ import {useEffect} from 'react'; +import {parseAsString, useQueryState} from 'nuqs'; import type {Event} from 'sentry/types/event'; import {getApiUrl} from 'sentry/utils/api/getApiUrl'; import {useApiQuery} from 'sentry/utils/queryClient'; -import {decodeScalar} from 'sentry/utils/queryString'; -import {useLocationQuery} from 'sentry/utils/url/useLocationQuery'; import {useNavigate} from 'sentry/utils/useNavigate'; import {useOrganization} from 'sentry/utils/useOrganization'; import {makeFeedbackPathname} from 'sentry/views/feedback/pathnames'; @@ -13,20 +12,16 @@ export function useRedirectToFeedbackFromEvent() { const organization = useOrganization(); const navigate = useNavigate(); - const {eventId, projectSlug} = useLocationQuery({ - fields: { - eventId: decodeScalar, - projectSlug: decodeScalar, - }, - }); + const [eventId] = useQueryState('eventId', parseAsString); + const [projectSlug] = useQueryState('projectSlug', parseAsString); const {data: event} = useApiQuery( [ getApiUrl('/projects/$organizationIdOrSlug/$projectIdOrSlug/events/$eventId/', { path: { organizationIdOrSlug: organization.slug, - projectIdOrSlug: projectSlug, - eventId, + projectIdOrSlug: projectSlug ?? '', + eventId: eventId ?? '', }, }), ], diff --git a/static/app/endpoints/organizations/organizationsConfigIntegrationsQueryOptions.ts b/static/app/components/repositories/scmIntegrationTree/organizationConfigIntegrationsQueryOptions.ts similarity index 100% rename from static/app/endpoints/organizations/organizationsConfigIntegrationsQueryOptions.ts rename to static/app/components/repositories/scmIntegrationTree/organizationConfigIntegrationsQueryOptions.ts diff --git a/static/app/components/repositories/scmIntegrationTree/useScmIntegrationTreeData.ts b/static/app/components/repositories/scmIntegrationTree/useScmIntegrationTreeData.ts index 0d3f8a947e05c2..6b45d8736cc24b 100644 --- a/static/app/components/repositories/scmIntegrationTree/useScmIntegrationTreeData.ts +++ b/static/app/components/repositories/scmIntegrationTree/useScmIntegrationTreeData.ts @@ -1,8 +1,7 @@ import {useEffect, useMemo} from 'react'; import {organizationRepositoriesInfiniteOptions} from 'sentry/components/events/autofix/preferences/hooks/useOrganizationRepositories'; -import {organizationConfigIntegrationsQueryOptions} from 'sentry/endpoints/organizations/organizationsConfigIntegrationsQueryOptions'; -import {organizationIntegrationsQueryOptions} from 'sentry/endpoints/organizations/organizationsIntegrationsQueryOptions'; +import {organizationConfigIntegrationsQueryOptions} from 'sentry/components/repositories/scmIntegrationTree/organizationConfigIntegrationsQueryOptions'; import type { IntegrationProvider, IntegrationRepository, @@ -12,6 +11,7 @@ import type { import {apiOptions} from 'sentry/utils/api/apiOptions'; import {useInfiniteQuery, useQueries, useQuery} from 'sentry/utils/queryClient'; import {useOrganization} from 'sentry/utils/useOrganization'; +import {organizationIntegrationsQueryOptions} from 'sentry/views/settings/seer/overview/utils/organizationIntegrationsQueryOptions'; type ScmIntegrationTreeData = { connectedIdentifiers: Set; diff --git a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx index 1169b5a0b47200..470d816476078b 100644 --- a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx +++ b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx @@ -185,6 +185,7 @@ type UpdateTokenValueAction = { token: TokenResult; type: 'UPDATE_TOKEN_VALUE'; value: string; + op?: TermOperator; }; type MultiSelectFilterValueAction = { @@ -280,6 +281,32 @@ function deleteQueryTokens( }; } +function termOperatorToInternal(op: TermOperator): { + internalOp: TermOperator; + negated: boolean; +} { + const negated = + op === TermOperator.NOT_EQUAL || + op === TermOperator.DOES_NOT_CONTAIN || + op === TermOperator.DOES_NOT_START_WITH || + op === TermOperator.DOES_NOT_END_WITH; + + let internalOp: TermOperator; + if (op === TermOperator.DOES_NOT_CONTAIN) { + internalOp = TermOperator.CONTAINS; + } else if (op === TermOperator.DOES_NOT_START_WITH) { + internalOp = TermOperator.STARTS_WITH; + } else if (op === TermOperator.DOES_NOT_END_WITH) { + internalOp = TermOperator.ENDS_WITH; + } else if (op === TermOperator.NOT_EQUAL) { + internalOp = TermOperator.DEFAULT; + } else { + internalOp = op; + } + + return {negated, internalOp}; +} + export function modifyFilterOperatorQuery( query: string, token: TokenResult, @@ -289,23 +316,10 @@ export function modifyFilterOperatorQuery( return modifyFilterOperatorDate(query, token, newOperator); } + const {negated, internalOp} = termOperatorToInternal(newOperator); const newToken: TokenResult = {...token}; - newToken.negated = - newOperator === TermOperator.NOT_EQUAL || - newOperator === TermOperator.DOES_NOT_CONTAIN || - newOperator === TermOperator.DOES_NOT_START_WITH || - newOperator === TermOperator.DOES_NOT_END_WITH; - - if (newOperator === TermOperator.DOES_NOT_CONTAIN) { - newToken.operator = TermOperator.CONTAINS; - } else if (newOperator === TermOperator.DOES_NOT_START_WITH) { - newToken.operator = TermOperator.STARTS_WITH; - } else if (newOperator === TermOperator.DOES_NOT_END_WITH) { - newToken.operator = TermOperator.ENDS_WITH; - } else { - newToken.operator = - newOperator === TermOperator.NOT_EQUAL ? TermOperator.DEFAULT : newOperator; - } + newToken.negated = negated; + newToken.operator = internalOp; return replaceQueryToken(query, token, stringifyToken(newToken)); } @@ -605,7 +619,8 @@ function replaceTokensWithText( export function modifyFilterValue( query: string, token: TokenResult, - newValue: string + newValue: string, + newOp?: TermOperator ): string { if (isDateToken(token)) { return modifyFilterValueDate(query, token, newValue); @@ -614,7 +629,18 @@ export function modifyFilterValue( // stop the user from entering multiple wildcards by themselves newValue = newValue.replace(/\*\*+/g, '*'); - return replaceQueryToken(query, token.value, newValue); + // No operator change — just replace the value (existing behavior) + if (newOp === undefined) { + return replaceQueryToken(query, token.value, newValue); + } + + // Operator change — replace the entire filter token atomically + const {negated, internalOp} = termOperatorToInternal(newOp); + + const prefix = negated ? '!' : ''; + const keyStr = stringifyToken(token.key); + const replacement = `${prefix}${keyStr}:${internalOp}${newValue}`; + return replaceQueryToken(query, token, replacement); } function updateFilterMultipleValues( @@ -1049,7 +1075,7 @@ export function useQueryBuilderState({ case 'UPDATE_TOKEN_VALUE': return { ...state, - query: modifyFilterValue(state.query, action.token, action.value), + query: modifyFilterValue(state.query, action.token, action.value, action.op), }; case 'UPDATE_LOGIC_OPERATOR': return updateLogicOperator(state, action); diff --git a/static/app/components/searchQueryBuilder/index.spec.tsx b/static/app/components/searchQueryBuilder/index.spec.tsx index 8cd8cf77dded29..4e82b8c027c7c7 100644 --- a/static/app/components/searchQueryBuilder/index.spec.tsx +++ b/static/app/components/searchQueryBuilder/index.spec.tsx @@ -1089,21 +1089,43 @@ describe('SearchQueryBuilder', () => { await userEvent.click(await screen.findByRole('option', {name: 'Firefox'})); - // New token should have a value + // New token should have a value, and selecting from dropdown switches operator to "is" expect( screen.getByRole('row', { - name: `browser.name:${WildcardOperators.CONTAINS}Firefox`, + name: 'browser.name:Firefox', }) ).toBeInTheDocument(); // Now we call onChange expect(mockOnChange).toHaveBeenCalledTimes(1); expect(mockOnChange).toHaveBeenCalledWith( - `browser.name:${WildcardOperators.CONTAINS}Firefox`, + 'browser.name:Firefox', expect.anything() ); }); + it('does not switch operator to "is" when filter already has a value', async () => { + render( + , + {organization: {features: ['search-query-builder-input-flow-changes']}} + ); + + await userEvent.click( + screen.getByRole('button', {name: 'Edit value for filter: browser.name'}) + ); + await userEvent.click(await screen.findByRole('option', {name: 'Chrome'})); + + // Operator should remain "contains" since there was already a value + expect( + await screen.findByRole('row', { + name: `browser.name:${WildcardOperators.CONTAINS}[firefox,Chrome]`, + }) + ).toBeInTheDocument(); + }); + it('can add free text by typing', async () => { const mockOnSearch = jest.fn(); render(); @@ -1266,9 +1288,10 @@ describe('SearchQueryBuilder', () => { await userEvent.keyboard('{enter}'); await userEvent.click(screen.getByRole('option', {name: '[Filtered]'})); + // Selecting from dropdown switches operator from contains to "is" expect( await screen.findByRole('row', { - name: `message:${WildcardOperators.CONTAINS}"[Filtered]"`, + name: 'message:"[Filtered]"', }) ).toBeInTheDocument(); }); diff --git a/static/app/components/searchQueryBuilder/tokens/filter/valueCombobox.tsx b/static/app/components/searchQueryBuilder/tokens/filter/valueCombobox.tsx index ca22790f1d133d..ab8cdef9324ba4 100644 --- a/static/app/components/searchQueryBuilder/tokens/filter/valueCombobox.tsx +++ b/static/app/components/searchQueryBuilder/tokens/filter/valueCombobox.tsx @@ -749,7 +749,7 @@ export function SearchQueryBuilderValueCombobox({ ); const updateFilterValue = useCallback( - (value: string) => { + (value: string, op?: TermOperator) => { if (token.filter === FilterType.HAS) { const suggested = getSuggestedFilterKey(value); if (suggested) { @@ -793,6 +793,7 @@ export function SearchQueryBuilderValueCombobox({ type: 'UPDATE_TOKEN_VALUE', token, value: newValue, + op, }); if (newValue && newValue !== '""' && !ctrlKeyPressed) { @@ -809,6 +810,7 @@ export function SearchQueryBuilderValueCombobox({ getFilterValueType(token, fieldDefinition), replaceCommaSeparatedValue(inputValue, selectionIndex, escapeTagValue(value)) ), + op, }); if (!ctrlKeyPressed) { @@ -819,6 +821,7 @@ export function SearchQueryBuilderValueCombobox({ type: 'UPDATE_TOKEN_VALUE', token, value: cleanedValue, + op, }); onCommit(); } @@ -860,7 +863,17 @@ export function SearchQueryBuilderValueCombobox({ return; } - updateFilterValue(value); + // When selecting from dropdown with no existing value, switch from "contains" to "is" + let newOp: TermOperator | undefined; + if ( + token.operator === TermOperator.CONTAINS && + token.value.type === Token.VALUE_TEXT && + !token.value.value + ) { + newOp = token.negated ? TermOperator.NOT_EQUAL : TermOperator.DEFAULT; + } + + updateFilterValue(value, newOp); trackAnalytics('search.value_autocompleted', { ...analyticsData, filter_value: value, diff --git a/static/app/types/hooks.tsx b/static/app/types/hooks.tsx index 0f7246ac3a8a62..3f75c04634cc1b 100644 --- a/static/app/types/hooks.tsx +++ b/static/app/types/hooks.tsx @@ -1,5 +1,4 @@ import type {ButtonProps} from '@sentry/scraps/button'; -import type {SelectKey} from '@sentry/scraps/compactSelect'; import type {ChildrenRenderFn} from 'sentry/components/acl/feature'; import type {Guide} from 'sentry/components/assistant/types'; @@ -168,12 +167,6 @@ export type MembershipSettingsProps = { onSave: (previous: Organization, updated: Organization) => void; organization: Organization; }; -export type GithubInstallationInstallButtonProps = { - handleSubmit: (e: React.MouseEvent) => void; - hasSCMMultiOrg: boolean; - installationID: SelectKey; - isSaving: boolean; -}; type DashboardLimitProviderProps = { children: @@ -230,7 +223,6 @@ type ComponentHooks = { 'component:replay-onboarding-alert': () => React.ComponentType; 'component:replay-onboarding-cta': () => React.ComponentType; 'component:replay-settings-alert': () => React.ComponentType | null; - 'component:scm-multi-org-install-button': () => React.ComponentType; 'component:seer-beta-closing-alert': () => React.ComponentType; 'component:superuser-access-category': React.ComponentType; 'component:superuser-warning': React.ComponentType; diff --git a/static/app/types/organization.tsx b/static/app/types/organization.tsx index a3768d08a0921d..1123f243ba52c5 100644 --- a/static/app/types/organization.tsx +++ b/static/app/types/organization.tsx @@ -65,8 +65,8 @@ export interface Organization extends OrganizationSummary { dataScrubberDefaults: boolean; debugFilesRole: string; defaultCodeReviewTriggers: CodeReviewTrigger[]; - defaultCodingAgent: string | null | undefined; - defaultCodingAgentIntegrationId: number | null | undefined; + defaultCodingAgent: string | null; + defaultCodingAgentIntegrationId: string | number | null; defaultRole: string; enhancedPrivacy: boolean; eventsMemberAdmin: boolean; diff --git a/static/app/utils/analytics/workflowAnalyticsEvents.tsx b/static/app/utils/analytics/workflowAnalyticsEvents.tsx index 6ae8631903d086..b8f327c5608375 100644 --- a/static/app/utils/analytics/workflowAnalyticsEvents.tsx +++ b/static/app/utils/analytics/workflowAnalyticsEvents.tsx @@ -181,6 +181,11 @@ export type TeamInsightsEventParameters = { 'releases_list.click_add_release_health': { project_id: number; }; + 'supergroup.feedback_submitted': { + choice_selected: boolean; + supergroup_id: number; + user_id: string; + }; 'suspect_commit.feedback_submitted': { choice_selected: boolean; group_owner_id: number; @@ -259,5 +264,6 @@ export const workflowEventMap: Record = { 'releases_list.click_add_release_health': 'Releases List: Click Add Release Health', trace_timeline_clicked: 'Trace Timeline Clicked', trace_timeline_more_events_clicked: 'Trace Timeline More Events Clicked', + 'supergroup.feedback_submitted': 'Supergroup Feedback Submitted', 'suspect_commit.feedback_submitted': 'Suspect Commit Feedback Submitted', }; diff --git a/static/app/utils/array/procesInChunks.spec.ts b/static/app/utils/array/procesInChunks.spec.ts new file mode 100644 index 00000000000000..80a4618ae083b0 --- /dev/null +++ b/static/app/utils/array/procesInChunks.spec.ts @@ -0,0 +1,108 @@ +import {processInChunks} from 'sentry/utils/array/procesInChunks'; + +describe('processInChunks', () => { + it('returns an empty array for empty input', async () => { + const fn = jest.fn().mockResolvedValue('x'); + const results = await processInChunks({items: [], chunkSize: 3, fn}); + expect(results).toEqual([]); + expect(fn).not.toHaveBeenCalled(); + }); + + it('processes all items when count is less than chunkSize', async () => { + const fn = jest.fn((x: number) => Promise.resolve(x * 2)); + const results = await processInChunks({items: [1, 2], chunkSize: 5, fn}); + expect(results).toEqual([ + {status: 'fulfilled', value: 2}, + {status: 'fulfilled', value: 4}, + ]); + }); + + it('processes all items when count equals chunkSize exactly', async () => { + const fn = jest.fn((x: number) => Promise.resolve(x * 2)); + const results = await processInChunks({items: [1, 2, 3], chunkSize: 3, fn}); + expect(results).toHaveLength(3); + expect(results.every(r => r.status === 'fulfilled')).toBe(true); + }); + + it('processes all items across multiple chunks', async () => { + const fn = jest.fn((x: number) => Promise.resolve(x)); + const results = await processInChunks({ + items: [1, 2, 3, 4, 5], + chunkSize: 2, + fn, + }); + expect(results).toHaveLength(5); + expect(fn).toHaveBeenCalledTimes(5); + expect(results.map(r => (r.status === 'fulfilled' ? r.value : null))).toEqual([ + 1, 2, 3, 4, 5, + ]); + }); + + it('preserves result order matching input order', async () => { + // Simulate varying async latency: later items resolve faster + const fn = jest.fn( + (x: number) => + new Promise(resolve => setTimeout(() => resolve(x), (10 - x) * 10)) + ); + const results = await processInChunks({items: [1, 2, 3, 4, 5], chunkSize: 5, fn}); + expect(results.map(r => (r.status === 'fulfilled' ? r.value : null))).toEqual([ + 1, 2, 3, 4, 5, + ]); + }); + + it('processes chunks sequentially, not all at once', async () => { + const callOrder: number[] = []; + const fn = jest.fn((x: number) => { + callOrder.push(x); + return Promise.resolve(x); + }); + + await processInChunks({items: [1, 2, 3, 4, 5, 6], chunkSize: 2, fn}); + + // Each chunk of 2 must start only after the previous chunk completes. + // Because fn is synchronous here, within each chunk the call order is + // preserved and chunks are processed sequentially. + expect(callOrder).toEqual([1, 2, 3, 4, 5, 6]); + }); + + it('marks rejected items as rejected without stopping other items', async () => { + const fn = jest.fn((x: number) => + x === 3 ? Promise.reject(new Error('boom')) : Promise.resolve(x) + ); + const results = await processInChunks({items: [1, 2, 3, 4, 5], chunkSize: 5, fn}); + expect(results).toHaveLength(5); + expect(results[0]).toEqual({status: 'fulfilled', value: 1}); + expect(results[1]).toEqual({status: 'fulfilled', value: 2}); + expect(results[2]).toMatchObject({status: 'rejected', reason: expect.any(Error)}); + expect(results[3]).toEqual({status: 'fulfilled', value: 4}); + expect(results[4]).toEqual({status: 'fulfilled', value: 5}); + }); + + it('continues processing later chunks when an earlier chunk has failures', async () => { + const fn = jest.fn((x: number) => + x === 1 ? Promise.reject(new Error('first chunk error')) : Promise.resolve(x) + ); + // chunk 1: [1] (fails), chunk 2: [2, 3] (succeeds) + const results = await processInChunks({items: [1, 2, 3], chunkSize: 1, fn}); + expect(results).toHaveLength(3); + expect(results[0]).toMatchObject({status: 'rejected'}); + expect(results[1]).toEqual({status: 'fulfilled', value: 2}); + expect(results[2]).toEqual({status: 'fulfilled', value: 3}); + }); + + it('handles chunkSize of 1 by processing items one at a time', async () => { + const fn = jest.fn((x: number) => Promise.resolve(x)); + const results = await processInChunks({items: [10, 20, 30], chunkSize: 1, fn}); + expect(results).toHaveLength(3); + expect(results.map(r => (r.status === 'fulfilled' ? r.value : null))).toEqual([ + 10, 20, 30, + ]); + }); + + it('handles chunkSize larger than item count', async () => { + const fn = jest.fn((x: number) => Promise.resolve(x)); + const results = await processInChunks({items: [1, 2], chunkSize: 100, fn}); + expect(results).toHaveLength(2); + expect(fn).toHaveBeenCalledTimes(2); + }); +}); diff --git a/static/app/utils/array/procesInChunks.ts b/static/app/utils/array/procesInChunks.ts new file mode 100644 index 00000000000000..5bb4458b2598e1 --- /dev/null +++ b/static/app/utils/array/procesInChunks.ts @@ -0,0 +1,19 @@ +interface Props { + chunkSize: number; + fn: (item: Item) => Promise; + items: Item[]; +} + +export async function processInChunks({ + items, + chunkSize, + fn, +}: Props): Promise>> { + const results: Array> = []; + for (let i = 0; i < items.length; i += chunkSize) { + const chunk = items.slice(i, i + chunkSize); + const chunkResults = await Promise.allSettled(chunk.map(fn)); + results.push(...chunkResults); + } + return results; +} diff --git a/static/app/utils/discover/fields.tsx b/static/app/utils/discover/fields.tsx index 06fa349e0d32f3..470e9cdfeb6e6a 100644 --- a/static/app/utils/discover/fields.tsx +++ b/static/app/utils/discover/fields.tsx @@ -24,6 +24,7 @@ import { } from 'sentry/views/dashboards/widgetBuilder/releaseWidget/fields'; import {STARFISH_FIELDS} from 'sentry/views/insights/common/utils/constants'; import {STARFISH_AGGREGATION_FIELDS} from 'sentry/views/insights/constants'; +import {SpanFields} from 'sentry/views/insights/types'; import {CONDITIONS_ARGUMENTS, DiscoverDatasets, WEB_VITALS_QUALITY} from './types'; @@ -1404,6 +1405,10 @@ export function fieldAlignment( columnType?: ColumnValueType, metadata?: Record ): Alignments { + if (columnName === SpanFields.IS_STARRED_TRANSACTION) { + return 'right'; + } + let align: Alignments = 'left'; if (columnType) { diff --git a/static/app/utils/supergroup/useSuperGroups.tsx b/static/app/utils/supergroup/useSuperGroups.tsx index 664ea33df0a662..8385fb02ac158a 100644 --- a/static/app/utils/supergroup/useSuperGroups.tsx +++ b/static/app/utils/supergroup/useSuperGroups.tsx @@ -1,4 +1,4 @@ -import {useMemo} from 'react'; +import {useMemo, useRef} from 'react'; import {getApiUrl} from 'sentry/utils/api/getApiUrl'; import {useApiQuery} from 'sentry/utils/queryClient'; @@ -17,8 +17,24 @@ export function useSuperGroups(groupIds: string[]): { isLoading: boolean; } { const organization = useOrganization(); + const requestedGroupIdsRef = useRef(groupIds); const hasTopIssuesUI = organization.features.includes('top-issues-ui'); - const enabled = hasTopIssuesUI && groupIds.length > 0; + const shouldReuseRequestedGroupIds = useMemo(() => { + const requestedGroupIds = requestedGroupIdsRef.current; + + if (groupIds.length === 0 || requestedGroupIds.length < groupIds.length) { + return false; + } + + const requestedGroupIdSet = new Set(requestedGroupIds); + return groupIds.every(groupId => requestedGroupIdSet.has(groupId)); + }, [groupIds]); + + const requestedGroupIds = shouldReuseRequestedGroupIds + ? requestedGroupIdsRef.current + : groupIds; + requestedGroupIdsRef.current = requestedGroupIds; + const enabled = hasTopIssuesUI && requestedGroupIds.length > 0; const {data: response, isLoading} = useApiQuery<{data: SupergroupDetail[]}>( [ @@ -27,7 +43,7 @@ export function useSuperGroups(groupIds: string[]): { }), { query: { - group_id: groupIds, + group_id: requestedGroupIds, }, }, ], diff --git a/static/app/views/automations/components/automationForm.tsx b/static/app/views/automations/components/automationForm.tsx index 7db57e6b38eda9..7d3a175d0d294f 100644 --- a/static/app/views/automations/components/automationForm.tsx +++ b/static/app/views/automations/components/automationForm.tsx @@ -1,12 +1,12 @@ import {useCallback} from 'react'; import {Flex} from '@sentry/scraps/layout'; -import {Heading, Text} from '@sentry/scraps/text'; import type {FormModel} from 'sentry/components/forms/model'; import {EnvironmentSelector} from 'sentry/components/workflowEngine/form/environmentSelector'; import {useFormField} from 'sentry/components/workflowEngine/form/useFormField'; import {Card} from 'sentry/components/workflowEngine/ui/card'; +import {Section} from 'sentry/components/workflowEngine/ui/section'; import {t} from 'sentry/locale'; import type {Automation} from 'sentry/types/workflowEngine/automations'; import {AutomationBuilder} from 'sentry/views/automations/components/automationBuilder'; @@ -32,34 +32,27 @@ export function AutomationForm({model}: {model: FormModel}) { setConnectedIds={setConnectedIds} /> - - - {t('Choose Environment')} - - - {t( - 'If you select environments different than your monitors then the automation will not fire.' - )} - - - +
+ +
- - {t('Alert Builder')} - - +
+ +
- - - {t('Action Interval')} - - - {t('Perform the actions above this often for an issue.')} - - - +
+ +
); diff --git a/static/app/views/dashboards/widgetCard/chart.tsx b/static/app/views/dashboards/widgetCard/chart.tsx index 0526a6108f1bec..5efdb037e42ac8 100644 --- a/static/app/views/dashboards/widgetCard/chart.tsx +++ b/static/app/views/dashboards/widgetCard/chart.tsx @@ -96,6 +96,7 @@ import {Thresholds as ThresholdsPlottable} from 'sentry/views/dashboards/widgets import {WheelWidgetVisualization} from 'sentry/views/dashboards/widgets/wheelWidget/wheelWidgetVisualization'; import {Actions} from 'sentry/views/discover/table/cellAction'; import {decodeColumnOrder} from 'sentry/views/discover/utils'; +import {SpanFields} from 'sentry/views/insights/types'; import type {SpanResponse} from 'sentry/views/insights/types'; import {WidgetCardConfidenceFooter} from './confidenceFooter'; @@ -614,7 +615,9 @@ function TableComponent({ tableResults[i]?.meta ).map((column, index) => { let sortable = false; - if (widget.widgetType === WidgetType.RELEASE) { + if (column.key === SpanFields.IS_STARRED_TRANSACTION) { + sortable = false; + } else if (widget.widgetType === WidgetType.RELEASE) { sortable = isAggregateField(column.key); } else if (widget.widgetType !== WidgetType.ISSUE) { sortable = true; diff --git a/static/app/views/dashboards/widgets/tableWidget/tableWidgetVisualization.spec.tsx b/static/app/views/dashboards/widgets/tableWidget/tableWidgetVisualization.spec.tsx index ac7e195a34dbdb..63985b776dd2c8 100644 --- a/static/app/views/dashboards/widgets/tableWidget/tableWidgetVisualization.spec.tsx +++ b/static/app/views/dashboards/widgets/tableWidget/tableWidgetVisualization.spec.tsx @@ -63,6 +63,43 @@ describe('TableWidgetVisualization', () => { expect($headers[1]).toHaveTextContent(columns[1]!.key!); }); + it('Renders star icon for is_starred_transaction column without alias', () => { + const starredTableData: TabularData = { + data: [{is_starred_transaction: true, transaction: '/api/foo'}], + meta: { + fields: {is_starred_transaction: 'boolean', transaction: 'string'}, + units: {is_starred_transaction: null, transaction: null}, + }, + }; + + render(); + + const $headers = screen.getAllByRole('columnheader'); + expect($headers[0]).not.toHaveTextContent('is_starred_transaction'); + expect($headers[0]!.querySelector('svg')).toBeInTheDocument(); + }); + + it('Renders alias text instead of star icon when alias is provided', () => { + const starredTableData: TabularData = { + data: [{is_starred_transaction: true, transaction: '/api/foo'}], + meta: { + fields: {is_starred_transaction: 'boolean', transaction: 'string'}, + units: {is_starred_transaction: null, transaction: null}, + }, + }; + + render( + + ); + + const $headers = screen.getAllByRole('columnheader'); + expect($headers[0]).toHaveTextContent('Starred'); + expect($headers[0]!.querySelector('svg')).not.toBeInTheDocument(); + }); + it('Renders unique number fields correctly', async () => { const tableData: TabularData = { data: [{'span.duration': 123, failure_rate: 0.1, epm: 6}], diff --git a/static/app/views/dashboards/widgets/tableWidget/tableWidgetVisualization.tsx b/static/app/views/dashboards/widgets/tableWidget/tableWidgetVisualization.tsx index b4bfc3318b2470..03115d07cef596 100644 --- a/static/app/views/dashboards/widgets/tableWidget/tableWidgetVisualization.tsx +++ b/static/app/views/dashboards/widgets/tableWidget/tableWidgetVisualization.tsx @@ -5,6 +5,7 @@ import {Tooltip} from '@sentry/scraps/tooltip'; import {COL_WIDTH_UNDEFINED, GridEditable} from 'sentry/components/tables/gridEditable'; import {SortLink} from 'sentry/components/tables/gridEditable/sortLink'; +import {IconStar} from 'sentry/icons'; import {defined} from 'sentry/utils'; import {getSortField} from 'sentry/utils/dashboards/issueFieldRenderers'; import type {TableDataRow} from 'sentry/utils/discover/discoverQuery'; @@ -35,6 +36,7 @@ import { CellAction, copyToClipboard, } from 'sentry/views/discover/table/cellAction'; +import {SpanFields} from 'sentry/views/insights/types'; export type FieldRendererGetter = ( field: string, @@ -233,9 +235,16 @@ export function TableWidgetVisualization(props: TableWidgetVisualizationProps) { grid={{ renderHeadCell: (_tableColumn, columnIndex) => { const column = columnOrder[columnIndex]!; + const isStarredColumn = column.key === SpanFields.IS_STARRED_TRANSACTION; + const hasAlias = !!aliases?.[column.key]; const align = fieldAlignment(column.key, column.type as ColumnValueType); - let name = aliases?.[column.key] || column.key; - if (isEquation(column.key)) name = stripEquationPrefix(name); + let name: React.ReactNode = aliases?.[column.key] || column.key; + if (isStarredColumn && !hasAlias) { + name = ; + } else if (isEquation(column.key)) { + name = stripEquationPrefix(name as string); + } + const tooltipTitle = isStarredColumn && !hasAlias ? column.key : name; const sortColumn = getSortField(column.key) ?? column.key; let direction = undefined; @@ -249,7 +258,7 @@ export function TableWidgetVisualization(props: TableWidgetVisualizationProps) { {name}} + title={{name}} onClick={e => { if (!onChangeSort) return; e.preventDefault(); @@ -376,14 +385,22 @@ TableWidgetVisualization.LoadingPlaceholder = function ({ renderHeadCell: (_tableColumn, columnIndex) => { if (!columns) return null; const column = columns[columnIndex]!; + const isStarredColumn = column.key === SpanFields.IS_STARRED_TRANSACTION; + const hasAlias = !!aliases?.[column.key]; const align = fieldAlignment(column.key, column.type as ColumnValueType); - const name = aliases?.[column.key] || column.key; + const displayAsIcon = isStarredColumn && !hasAlias; + const name: React.ReactNode = displayAsIcon ? ( + + ) : ( + aliases?.[column.key] || column.key + ); + const tooltipTitle = displayAsIcon ? column.key : name; return ( {name}} + title={{name}} direction={undefined} generateSortLink={() => undefined} /> diff --git a/static/app/views/detectors/list/allMonitors.tsx b/static/app/views/detectors/list/allMonitors.tsx index 58d95c75695d3d..e9b38236c92529 100644 --- a/static/app/views/detectors/list/allMonitors.tsx +++ b/static/app/views/detectors/list/allMonitors.tsx @@ -19,7 +19,7 @@ export default function AllMonitors() { return ( } + actions={} title={TITLE} description={DESCRIPTION} docsUrl={DOCS_URL} diff --git a/static/app/views/detectors/list/common/detectorListActions.tsx b/static/app/views/detectors/list/common/detectorListActions.tsx index db3724911f498e..0a14dccd1cdd07 100644 --- a/static/app/views/detectors/list/common/detectorListActions.tsx +++ b/static/app/views/detectors/list/common/detectorListActions.tsx @@ -5,42 +5,22 @@ import {ALL_ACCESS_PROJECTS} from 'sentry/components/pageFilters/constants'; import {usePageFilters} from 'sentry/components/pageFilters/usePageFilters'; import {IconAdd} from 'sentry/icons'; import {t} from 'sentry/locale'; -import type {DetectorType} from 'sentry/types/workflowEngine/detectors'; import {useOrganization} from 'sentry/utils/useOrganization'; import {MonitorFeedbackButton} from 'sentry/views/detectors/components/monitorFeedbackButton'; -import { - makeMonitorCreatePathname, - makeMonitorCreateSettingsPathname, -} from 'sentry/views/detectors/pathnames'; -import {detectorTypeIsUserCreateable} from 'sentry/views/detectors/utils/detectorTypeConfig'; +import {makeMonitorCreatePathname} from 'sentry/views/detectors/pathnames'; import {getNoPermissionToCreateMonitorsTooltip} from 'sentry/views/detectors/utils/monitorAccessMessages'; import {useCanCreateDetector} from 'sentry/views/detectors/utils/useCanCreateDetector'; interface DetectorListActionsProps { - detectorType: DetectorType | null; children?: React.ReactNode; } -function getPermissionTooltipText({detectorType}: {detectorType: DetectorType | null}) { - const noPermissionText = getNoPermissionToCreateMonitorsTooltip(); - - if (!detectorType || detectorTypeIsUserCreateable(detectorType)) { - return noPermissionText; - } - - return t('This monitor type is managed by Sentry.'); -} - -export function DetectorListActions({detectorType, children}: DetectorListActionsProps) { +export function DetectorListActions({children}: DetectorListActionsProps) { const organization = useOrganization(); const {selection} = usePageFilters(); - const createPath = detectorType - ? makeMonitorCreateSettingsPathname(organization.slug) - : makeMonitorCreatePathname(organization.slug); const project = selection.projects.find(pid => pid !== ALL_ACCESS_PROJECTS); - const createQuery = detectorType ? {project, detectorType} : {project}; - const canCreateDetector = useCanCreateDetector(detectorType); + const canCreateDetector = useCanCreateDetector(null); return ( @@ -48,19 +28,15 @@ export function DetectorListActions({detectorType, children}: DetectorListAction } size="sm" disabled={!canCreateDetector} tooltipProps={{ - title: canCreateDetector - ? undefined - : getPermissionTooltipText({ - detectorType, - }), + title: canCreateDetector ? undefined : getNoPermissionToCreateMonitorsTooltip(), }} > {t('Create Monitor')} diff --git a/static/app/views/detectors/list/cron.tsx b/static/app/views/detectors/list/cron.tsx index ac861c9fa7b39e..d291cfe7f475fc 100644 --- a/static/app/views/detectors/list/cron.tsx +++ b/static/app/views/detectors/list/cron.tsx @@ -244,7 +244,7 @@ export default function CronDetectorsList() { } + actions={} title={TITLE} description={DESCRIPTION} docsUrl={DOCS_URL} diff --git a/static/app/views/detectors/list/error.tsx b/static/app/views/detectors/list/error.tsx index d2ed71479687aa..cc23e6a3360d38 100644 --- a/static/app/views/detectors/list/error.tsx +++ b/static/app/views/detectors/list/error.tsx @@ -21,7 +21,7 @@ export default function ErrorDetectorsList() { return ( } + actions={} title={TITLE} description={DESCRIPTION} docsUrl={DOCS_URL} diff --git a/static/app/views/detectors/list/metric.tsx b/static/app/views/detectors/list/metric.tsx index 063e6bfcf0d58f..c0de7b94a40932 100644 --- a/static/app/views/detectors/list/metric.tsx +++ b/static/app/views/detectors/list/metric.tsx @@ -21,7 +21,7 @@ export default function MetricDetectorsList() { return ( } + actions={} title={TITLE} description={DESCRIPTION} docsUrl={DOCS_URL} diff --git a/static/app/views/detectors/list/mobileBuild.tsx b/static/app/views/detectors/list/mobileBuild.tsx index daea09353a6e25..fe6914cf811e13 100644 --- a/static/app/views/detectors/list/mobileBuild.tsx +++ b/static/app/views/detectors/list/mobileBuild.tsx @@ -22,7 +22,7 @@ export default function MobileBuildDetectorsList() { } + actions={} title={TITLE} description={DESCRIPTION} docsUrl={DOCS_URL} diff --git a/static/app/views/detectors/list/myMonitors.tsx b/static/app/views/detectors/list/myMonitors.tsx index 2d81d8172bf5a4..6c91043f495f20 100644 --- a/static/app/views/detectors/list/myMonitors.tsx +++ b/static/app/views/detectors/list/myMonitors.tsx @@ -18,7 +18,7 @@ export default function MyMonitorsList() { return ( } + actions={} title={TITLE} description={DESCRIPTION} docsUrl={DOCS_URL} diff --git a/static/app/views/detectors/list/uptime.tsx b/static/app/views/detectors/list/uptime.tsx index fa385dbfc4222b..dbbacafdcd3285 100644 --- a/static/app/views/detectors/list/uptime.tsx +++ b/static/app/views/detectors/list/uptime.tsx @@ -105,7 +105,7 @@ export default function UptimeDetectorsList() { } + actions={} title={TITLE} description={DESCRIPTION} docsUrl={DOCS_URL} diff --git a/static/app/views/detectors/pathnames.tsx b/static/app/views/detectors/pathnames.tsx index c89431355cbe4d..33b54650b84f2b 100644 --- a/static/app/views/detectors/pathnames.tsx +++ b/static/app/views/detectors/pathnames.tsx @@ -23,10 +23,6 @@ export const makeMonitorCreatePathname = (orgSlug: string) => { return normalizeUrl(`${makeMonitorBasePathname(orgSlug)}new/`); }; -export const makeMonitorCreateSettingsPathname = (orgSlug: string) => { - return normalizeUrl(`${makeMonitorBasePathname(orgSlug)}new/settings/`); -}; - export const makeMonitorEditPathname = (orgSlug: string, monitorId: string) => { return normalizeUrl(`${makeMonitorBasePathname(orgSlug)}${monitorId}/edit/`); }; diff --git a/static/app/views/explore/metrics/metricToolbar/aggregateDropdown.tsx b/static/app/views/explore/metrics/metricToolbar/aggregateDropdown.tsx index d308eea86c3bef..ab1dc14f95dd1a 100644 --- a/static/app/views/explore/metrics/metricToolbar/aggregateDropdown.tsx +++ b/static/app/views/explore/metrics/metricToolbar/aggregateDropdown.tsx @@ -48,10 +48,18 @@ export function AggregateDropdown({traceMetric}: {traceMetric: TraceMetric}) { } const selectedList = [...selectedNames].filter(Boolean); + const defaultValue = DEFAULT_YAXIS_BY_TYPE[traceMetric.type]; + const isDefaultSelection = + selectedList.length === 1 && selectedList[0] === defaultValue; return ( handleChange([])} /> + } style={{width: '100%'}} trigger={triggerProps => ( >) => handleChange(opts)} + onChange={handleChange} /> ); } @@ -100,10 +108,10 @@ export function AggregateDropdown({traceMetric}: {traceMetric: TraceMetric}) { return ( ) => handleChange([opt])} + value={activeValues[0]} + onChange={opt => handleChange([opt])} /> ); })} diff --git a/static/app/views/feedback/feedbackListPage.tsx b/static/app/views/feedback/feedbackListPage.tsx index 7ec776ba920d7c..6660330939780b 100644 --- a/static/app/views/feedback/feedbackListPage.tsx +++ b/static/app/views/feedback/feedbackListPage.tsx @@ -13,10 +13,9 @@ import {FeedbackSearch} from 'sentry/components/feedback/feedbackSearch'; import {FeedbackSetupPanel} from 'sentry/components/feedback/feedbackSetupPanel'; import {FeedbackList} from 'sentry/components/feedback/list/feedbackList'; import {FeedbackSummaryCategories} from 'sentry/components/feedback/summaryCategories/feedbackSummaryCategories'; -import {useCurrentFeedbackId} from 'sentry/components/feedback/useCurrentFeedbackId'; -import {useCurrentFeedbackProject} from 'sentry/components/feedback/useCurrentFeedbackProject'; import {useHaveSelectedProjectsSetupFeedback} from 'sentry/components/feedback/useFeedbackOnboarding'; import {FeedbackQueryKeys} from 'sentry/components/feedback/useFeedbackQueryKeys'; +import {useFeedbackSlug} from 'sentry/components/feedback/useFeedbackSlug'; import {useRedirectToFeedbackFromEvent} from 'sentry/components/feedback/useRedirectToFeedbackFromEvent'; import {FeedbackButton} from 'sentry/components/feedbackButton/feedbackButton'; import {FullViewport} from 'sentry/components/layouts/fullViewport'; @@ -37,8 +36,9 @@ export default function FeedbackListPage() { const {hasSetupOneFeedback} = useHaveSelectedProjectsSetupFeedback(); const pageFilters = usePageFilters(); - const feedbackId = useCurrentFeedbackId(); - const feedbackProjectSlug = useCurrentFeedbackProject(); + const [feedbackSlug] = useFeedbackSlug(); + const feedbackId = feedbackSlug?.feedbackId ?? ''; + const feedbackProjectSlug = feedbackSlug?.projectSlug ?? ''; const hasSlug = Boolean(feedbackId); const {query: locationQuery} = useLocation(); @@ -175,6 +175,7 @@ export default function FeedbackListPage() { query: { alert_option: 'issues', referrer: 'feedback-list-page', + detectorType: 'metric_issue', ...(feedbackProjectSlug ? {project: feedbackProjectSlug} : {}), }, }} diff --git a/static/app/views/integrationPipeline/githubInstallationSelect.tsx b/static/app/views/integrationPipeline/githubInstallationSelect.tsx index 17ff7315043547..6bb85ad57665ea 100644 --- a/static/app/views/integrationPipeline/githubInstallationSelect.tsx +++ b/static/app/views/integrationPipeline/githubInstallationSelect.tsx @@ -11,11 +11,9 @@ import {OverlayTrigger} from '@sentry/scraps/overlayTrigger'; import {addLoadingMessage} from 'sentry/actionCreators/indicator'; import {FeatureDisabled} from 'sentry/components/acl/featureDisabled'; -import {HookOrDefault} from 'sentry/components/hookOrDefault'; import {IconAdd, IconLightning} from 'sentry/icons'; import {t} from 'sentry/locale'; import {ConfigStore} from 'sentry/stores/configStore'; -import type {GithubInstallationInstallButtonProps} from 'sentry/types/hooks'; import type {Organization} from 'sentry/types/organization'; import {testableWindowLocation} from 'sentry/utils/testableWindowLocation'; @@ -31,38 +29,6 @@ type GithubInstallationProps = { organization: Organization; }; -function InstallationButton({ - handleSubmit, - isSaving, - installationID, - hasSCMMultiOrg, -}: GithubInstallationInstallButtonProps) { - if (installationID !== '-1' && !hasSCMMultiOrg) { - return ( - - ); - } - - return ( - - ); -} - -const InstallButtonHook = HookOrDefault({ - hookName: 'component:scm-multi-org-install-button', - defaultComponent: InstallationButton, -}); - export function GithubInstallationSelect({ installation_info, organization, @@ -193,20 +159,7 @@ export function GithubInstallationSelect({ )} /> - {organization.features.includes('github-multi-org-upsell-modal') ? ( - i.installation_id === installationID) - ?.count === 0 - } - installationID={installationID} - isSaving={isSaving} - handleSubmit={handleSubmit} - /> - ) : ( - renderInstallationButtonOld() - )} + {renderInstallationButtonOld()} diff --git a/static/app/views/issueDetails/groupSimilarIssues/similarIssues.spec.tsx b/static/app/views/issueDetails/groupSimilarIssues/similarIssues.spec.tsx index 7edc2f5687c926..23874bd43af7d2 100644 --- a/static/app/views/issueDetails/groupSimilarIssues/similarIssues.spec.tsx +++ b/static/app/views/issueDetails/groupSimilarIssues/similarIssues.spec.tsx @@ -216,7 +216,7 @@ describe('Issues Similar Embeddings View', () => { beforeEach(() => { mock = MockApiClient.addMockResponse({ - url: `/organizations/org-slug/issues/${group.id}/similar-issues-embeddings/?k=10&threshold=0.01&useReranking=true`, + url: `/organizations/org-slug/issues/${group.id}/similar-issues-embeddings/?k=10&threshold=0.01`, body: mockData.similarEmbeddings, }); MockApiClient.addMockResponse({ @@ -330,7 +330,7 @@ describe('Issues Similar Embeddings View', () => { it('shows empty message', async () => { mock = MockApiClient.addMockResponse({ - url: `/organizations/org-slug/issues/${group.id}/similar-issues-embeddings/?k=10&threshold=0.01&useReranking=true`, + url: `/organizations/org-slug/issues/${group.id}/similar-issues-embeddings/?k=10&threshold=0.01`, body: [], }); diff --git a/static/app/views/issueDetails/groupSimilarIssues/similarStackTrace/index.tsx b/static/app/views/issueDetails/groupSimilarIssues/similarStackTrace/index.tsx index f48a2bc068a65d..8de2cdb88da906 100644 --- a/static/app/views/issueDetails/groupSimilarIssues/similarStackTrace/index.tsx +++ b/static/app/views/issueDetails/groupSimilarIssues/similarStackTrace/index.tsx @@ -58,11 +58,6 @@ export function SimilarStackTrace({project}: Props) { const hasSimilarityEmbeddingsFeature = projectData?.features.includes('similarity-embeddings') || location.query.similarityEmbeddings === '1'; - // Use reranking by default (assuming the `seer.similarity.similar_issues.use_reranking` - // backend option is using its default value of `True`). This is just so we can turn it off - // on demand to see if/how that changes the results. - const useReranking = String(location.query.useReranking !== '0'); - const fetchData = useCallback(() => { if (isPending) { return; @@ -77,7 +72,6 @@ export function SimilarStackTrace({project}: Props) { { k: 10, threshold: 0.01, - useReranking, } )}`, dataKey: 'similar', @@ -101,7 +95,6 @@ export function SimilarStackTrace({project}: Props) { organization.slug, hasSimilarityFeature, hasSimilarityEmbeddingsFeature, - useReranking, isPending, ]); diff --git a/static/app/views/issueList/supergroups/supergroupDrawer.tsx b/static/app/views/issueList/supergroups/supergroupDrawer.tsx index 885bdd9524464a..66f378c2bbe65b 100644 --- a/static/app/views/issueList/supergroups/supergroupDrawer.tsx +++ b/static/app/views/issueList/supergroups/supergroupDrawer.tsx @@ -1,8 +1,8 @@ import {Fragment} from 'react'; import styled from '@emotion/styled'; +import {Badge} from '@sentry/scraps/badge'; import {Container, Flex, Stack} from '@sentry/scraps/layout'; -import {Link} from '@sentry/scraps/link'; import {Heading, Text} from '@sentry/scraps/text'; import { @@ -15,12 +15,11 @@ import {GroupList} from 'sentry/components/issues/groupList'; import {ALL_ACCESS_PROJECTS} from 'sentry/components/pageFilters/constants'; import {IconFocus} from 'sentry/icons'; import {t} from 'sentry/locale'; -import {useOrganization} from 'sentry/utils/useOrganization'; import {StyledMarkedText} from 'sentry/views/issueList/pages/supergroups'; +import {SupergroupFeedback} from 'sentry/views/issueList/supergroups/supergroupFeedback'; import type {SupergroupDetail} from 'sentry/views/issueList/supergroups/types'; export function SupergroupDetailDrawer({supergroup}: {supergroup: SupergroupDetail}) { - const organization = useOrganization(); const placeholderRows = Math.min(supergroup.group_ids.length, 10); const issueIdQuery = `issue.id:[${supergroup.group_ids.join(',')}]`; @@ -28,29 +27,25 @@ export function SupergroupDetailDrawer({supergroup}: {supergroup: SupergroupDeta - - {`SG-${supergroup.id}`} - - ), - }, - ]} - /> - - {t('View All Issues')} ({supergroup.group_ids.length}) - + + + {`SG-${supergroup.id}`} + + ), + }, + ]} + /> + {t('Experimental')} + + diff --git a/static/app/views/issueList/supergroups/supergroupFeedback.tsx b/static/app/views/issueList/supergroups/supergroupFeedback.tsx new file mode 100644 index 00000000000000..5af7397183ee43 --- /dev/null +++ b/static/app/views/issueList/supergroups/supergroupFeedback.tsx @@ -0,0 +1,70 @@ +import {useCallback, useState} from 'react'; +import styled from '@emotion/styled'; + +import {Button} from '@sentry/scraps/button'; +import {Flex} from '@sentry/scraps/layout'; + +import {IconThumb} from 'sentry/icons'; +import {t} from 'sentry/locale'; +import {trackAnalytics} from 'sentry/utils/analytics'; +import {useOrganization} from 'sentry/utils/useOrganization'; +import {useUser} from 'sentry/utils/useUser'; + +interface SupergroupFeedbackProps { + supergroupId: number; +} + +export function SupergroupFeedback({supergroupId}: SupergroupFeedbackProps) { + const [feedbackSubmitted, setFeedbackSubmitted] = useState(false); + const organization = useOrganization(); + const user = useUser(); + + const handleFeedback = useCallback( + (isAccurate: boolean) => { + trackAnalytics('supergroup.feedback_submitted', { + choice_selected: isAccurate, + supergroup_id: supergroupId, + user_id: user.id, + organization, + }); + + setFeedbackSubmitted(true); + }, + [supergroupId, organization, user] + ); + + return ( + + {feedbackSubmitted ? ( + t('Thanks!') + ) : ( + + {t('Help us improve this feature. Is this grouping accurate?')} + + {projects.length === 0 ? t('No projects found') : projects.length === 1 - ? projectsWithPreferredAgentCount === 1 + ? projectsWithPreferredAgent.length === 1 ? t('Your existing project uses %s', preferredAgentLabel) : t('Your existing project does not use %s', preferredAgentLabel) - : projects.length === projectsWithPreferredAgentCount + : projects.length === projectsWithPreferredAgent.length ? t('All existing projects use %s', preferredAgentLabel) : t( '%s of %s existing projects use %s', - projectsWithPreferredAgentCount, + projectsWithPreferredAgent.length, projects.length, preferredAgentLabel )} @@ -212,15 +280,22 @@ function AgentNameForm({ function CreatePrForm({ canWrite, + isPending, + isBulkMutatingCreatePr, + setIsBulkMutatingCreatePr, + isBulkMutatingAgent, organization, projects, - projectsWithCreatePrCount, + projectsWithCreatePr, }: { canWrite: boolean; + isBulkMutatingAgent: boolean; + isBulkMutatingCreatePr: boolean; isPending: boolean; organization: Organization; projects: Project[]; - projectsWithCreatePrCount: number; + projectsWithCreatePr: AutofixAutomationSettings[]; + setIsBulkMutatingCreatePr: (value: boolean) => void; }) { const orgMutationOpts = mutationOptions({ mutationFn: (updateData: Partial) => @@ -232,6 +307,11 @@ function CreatePrForm({ onSuccess: updateOrganization, }); + const projectsWithCreatePrIds = new Set(projectsWithCreatePr.map(s => s.projectId)); + const projectsToUpdate = projects.filter(p => !projectsWithCreatePrIds.has(p.id)); + + const bulkMutateCreatePr = useBulkMutateCreatePr({projects: projectsToUpdate}); + return ( + {projects.length === 0 ? t('No projects found') : projects.length === 1 - ? projectsWithCreatePrCount === 1 + ? projectsWithCreatePr.length === 1 ? t('Your existing project has Create PR enabled') : t('Your existing project does not have Create PR enabled') : field.state.value - ? projects.length === projectsWithCreatePrCount + ? projects.length === projectsWithCreatePr.length ? t('All existing projects have Create PR enabled') : t( '%s of %s existing projects have Create PR enabled', - projectsWithCreatePrCount, + projectsWithCreatePr.length, projects.length ) - : projects.length === projectsWithCreatePrCount + : projects.length === projectsWithCreatePr.length ? t('All existing projects have Create PR disabled') : t( '%s of %s existing projects have Create PR disabled', - projectsWithCreatePrCount, + projectsWithCreatePr.length, projects.length )} diff --git a/static/app/views/settings/seer/overview/scmOverviewSection.tsx b/static/app/views/settings/seer/overview/scmOverviewSection.tsx index 10047d9e64bee6..b898ebe03ca433 100644 --- a/static/app/views/settings/seer/overview/scmOverviewSection.tsx +++ b/static/app/views/settings/seer/overview/scmOverviewSection.tsx @@ -25,7 +25,6 @@ import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {RepoProviderIcon} from 'sentry/components/repositories/repoProviderIcon'; import {getProviderConfigUrl} from 'sentry/components/repositories/scmIntegrationTree/providerConfigLink'; import {useScmIntegrationTreeData} from 'sentry/components/repositories/scmIntegrationTree/useScmIntegrationTreeData'; -import {ScmRepoTreeModal} from 'sentry/components/repositories/scmRepoTreeModal'; import {IconAdd, IconOpen, IconSettings} from 'sentry/icons'; import {t} from 'sentry/locale'; import type { @@ -178,7 +177,10 @@ function NoIntegrations({refetchIntegrations}: {refetchIntegrations: () => void} priority="primary" size="sm" icon={} - onClick={() => { + onClick={async () => { + const {ScmRepoTreeModal} = + await import('sentry/components/repositories/scmRepoTreeModal'); + openModal( deps => , { diff --git a/static/app/endpoints/organizations/organizationsIntegrationsQueryOptions.ts b/static/app/views/settings/seer/overview/utils/organizationIntegrationsQueryOptions.ts similarity index 100% rename from static/app/endpoints/organizations/organizationsIntegrationsQueryOptions.ts rename to static/app/views/settings/seer/overview/utils/organizationIntegrationsQueryOptions.ts diff --git a/static/app/views/settings/seer/seerAgentHooks.spec.tsx b/static/app/views/settings/seer/seerAgentHooks.spec.tsx index d7a0f5eaedb191..bb72d9c17e7663 100644 --- a/static/app/views/settings/seer/seerAgentHooks.spec.tsx +++ b/static/app/views/settings/seer/seerAgentHooks.spec.tsx @@ -14,6 +14,8 @@ import {ProjectsStore} from 'sentry/stores/projectsStore'; import {useQueryClient} from 'sentry/utils/queryClient'; import { useAgentOptions, + useBulkMutateCreatePr, + useBulkMutateSelectedAgent, useMutateCreatePr, useMutateSelectedAgent, useSelectedAgentFromBulkSettings, @@ -35,7 +37,7 @@ describe('seerAgentHooks', () => { }); describe('useAgentOptions', () => { - it('returns Seer, integration options, and No Handoff Selection', () => { + it('returns Seer, integration options', () => { const integrations: CodingAgentIntegration[] = [ {id: '42', name: 'Cursor', provider: 'cursor'}, ]; @@ -616,6 +618,645 @@ describe('seerAgentHooks', () => { }); }); + describe('useBulkMutateSelectedAgent', () => { + const project1 = ProjectFixture({slug: 'project-slug', id: '1'}); + const project2 = ProjectFixture({slug: 'project-slug-2', id: '2'}); + const projects = [project1, project2]; + + const basePreference: ProjectSeerPreferences = { + repositories: [], + automated_run_stopping_point: 'code_changes', + automation_handoff: undefined, + }; + + function setupMocks(preference: ProjectSeerPreferences = basePreference) { + const mocks = projects.map(p => ({ + seerPreferencesGetRequest: MockApiClient.addMockResponse({ + url: `/projects/${organization.slug}/${p.slug}/seer/preferences/`, + method: 'GET', + body: {preference, code_mapping_repos: []} satisfies SeerPreferencesResponse, + }), + projectPutRequest: MockApiClient.addMockResponse({ + url: `/projects/${organization.slug}/${p.slug}/`, + method: 'PUT', + body: p, + }), + seerPreferencesPostRequest: MockApiClient.addMockResponse({ + url: `/projects/${organization.slug}/${p.slug}/seer/preferences/`, + method: 'POST', + body: {}, + }), + })); + return { + seerPreferencesGetRequests: mocks.map(m => m.seerPreferencesGetRequest), + projectPutRequests: mocks.map(m => m.projectPutRequest), + seerPreferencesPostRequests: mocks.map(m => m.seerPreferencesPostRequest), + }; + } + + function renderBulkMutateSelectedAgent() { + return renderHookWithProviders( + (props: {projects: typeof projects}) => { + const mutate = useBulkMutateSelectedAgent(props); + return {mutate}; + }, + { + initialProps: {projects}, + organization, + } + ); + } + + beforeEach(() => { + ProjectsStore.loadInitialData(projects); + }); + + it('sends correct API requests to all projects when integration is "seer"', async () => { + const {projectPutRequests, seerPreferencesPostRequests} = setupMocks(); + const {result} = renderBulkMutateSelectedAgent(); + + act(() => { + result.current.mutate('seer', {}); + }); + + await waitFor(() => { + expect(projectPutRequests[0]).toHaveBeenCalledTimes(1); + }); + + projects.forEach((p, i) => { + expect(projectPutRequests[i]).toHaveBeenCalledWith( + `/projects/${organization.slug}/${p.slug}/`, + expect.objectContaining({ + method: 'PUT', + data: {autofixAutomationTuning: 'medium'}, + }) + ); + expect(seerPreferencesPostRequests[i]).toHaveBeenCalledWith( + `/projects/${organization.slug}/${p.slug}/seer/preferences/`, + expect.objectContaining({ + method: 'POST', + data: expect.objectContaining({ + repositories: [], + automated_run_stopping_point: 'code_changes', + automation_handoff: undefined, + }), + }) + ); + }); + }); + + it('sends correct API requests to all projects when integration is a CodingAgentIntegration', async () => { + const {projectPutRequests, seerPreferencesPostRequests} = setupMocks(); + const integration: CodingAgentIntegration = { + id: '123', + name: 'Cursor', + provider: 'cursor', + }; + const {result} = renderBulkMutateSelectedAgent(); + + act(() => { + result.current.mutate(integration, {}); + }); + + await waitFor(() => { + expect(projectPutRequests[0]).toHaveBeenCalledTimes(1); + }); + + projects.forEach((p, i) => { + expect(projectPutRequests[i]).toHaveBeenCalledWith( + `/projects/${organization.slug}/${p.slug}/`, + expect.objectContaining({ + method: 'PUT', + data: {autofixAutomationTuning: 'medium'}, + }) + ); + expect(seerPreferencesPostRequests[i]).toHaveBeenCalledWith( + `/projects/${organization.slug}/${p.slug}/seer/preferences/`, + expect.objectContaining({ + method: 'POST', + data: expect.objectContaining({ + automation_handoff: { + handoff_point: 'root_cause', + target: 'cursor_background_agent', + integration_id: 123, + auto_create_pr: false, + }, + }), + }) + ); + }); + }); + + it('sets auto_create_pr true when preference stopping point is open_pr', async () => { + const {seerPreferencesPostRequests} = setupMocks({ + ...basePreference, + automated_run_stopping_point: 'open_pr', + }); + const integration: CodingAgentIntegration = { + id: '456', + name: 'Cursor', + provider: 'cursor', + }; + const {result} = renderBulkMutateSelectedAgent(); + + act(() => { + result.current.mutate(integration, {}); + }); + + await waitFor(() => { + expect(seerPreferencesPostRequests[0]).toHaveBeenCalledTimes(1); + }); + + projects.forEach((_p, i) => { + expect(seerPreferencesPostRequests[i]).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + data: expect.objectContaining({ + automation_handoff: expect.objectContaining({ + auto_create_pr: true, + }), + }), + }) + ); + }); + }); + + it('preserves repositories from each project preference', async () => { + const preferenceWithRepos: ProjectSeerPreferences = { + repositories: [ + {external_id: 'repo-1', name: 'my-repo', owner: 'my-org', provider: 'github'}, + ], + automated_run_stopping_point: 'code_changes', + automation_handoff: undefined, + }; + const {seerPreferencesPostRequests} = setupMocks(preferenceWithRepos); + const {result} = renderBulkMutateSelectedAgent(); + + act(() => { + result.current.mutate('seer', {}); + }); + + await waitFor(() => { + expect(seerPreferencesPostRequests[0]).toHaveBeenCalledTimes(1); + }); + + projects.forEach((_p, i) => { + expect(seerPreferencesPostRequests[i]).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + data: expect.objectContaining({ + repositories: [ + { + external_id: 'repo-1', + name: 'my-repo', + owner: 'my-org', + provider: 'github', + }, + ], + }), + }) + ); + }); + }); + + it('updates ProjectsStore for all projects', async () => { + setupMocks(); + const storeSpy = jest.spyOn(ProjectsStore, 'onUpdateSuccess'); + const {result} = renderBulkMutateSelectedAgent(); + + act(() => { + result.current.mutate('seer', {}); + }); + + await waitFor(() => { + expect(storeSpy).toHaveBeenCalledTimes(2); + }); + + projects.forEach(p => { + expect(storeSpy).toHaveBeenCalledWith({ + id: p.id, + autofixAutomationTuning: 'medium', + }); + }); + }); + + it('updates ProjectsStore with "off" tuning for all projects when integration is "none"', async () => { + setupMocks(); + const storeSpy = jest.spyOn(ProjectsStore, 'onUpdateSuccess'); + const {result} = renderBulkMutateSelectedAgent(); + + act(() => { + result.current.mutate('none', {}); + }); + + await waitFor(() => { + expect(storeSpy).toHaveBeenCalledTimes(2); + }); + + projects.forEach(p => { + expect(storeSpy).toHaveBeenCalledWith({ + id: p.id, + autofixAutomationTuning: 'off', + }); + }); + }); + + it('calls onSuccess when all requests succeed', async () => { + setupMocks(); + const onSuccess = jest.fn(); + const {result} = renderBulkMutateSelectedAgent(); + + act(() => { + result.current.mutate('seer', {onSuccess}); + }); + + await waitFor(() => { + expect(onSuccess).toHaveBeenCalledTimes(1); + }); + }); + + it('calls onError when any request fails', async () => { + projects.forEach(p => { + MockApiClient.addMockResponse({ + url: `/projects/${organization.slug}/${p.slug}/seer/preferences/`, + method: 'GET', + body: { + preference: basePreference, + code_mapping_repos: [], + } satisfies SeerPreferencesResponse, + }); + MockApiClient.addMockResponse({ + url: `/projects/${organization.slug}/${p.slug}/`, + method: 'PUT', + statusCode: 500, + body: {}, + }); + MockApiClient.addMockResponse({ + url: `/projects/${organization.slug}/${p.slug}/seer/preferences/`, + method: 'POST', + body: {}, + }); + }); + const onError = jest.fn(); + const {result} = renderBulkMutateSelectedAgent(); + + act(() => { + result.current.mutate('seer', {onError}); + }); + + await waitFor(() => { + expect(onError).toHaveBeenCalledTimes(1); + }); + expect(onError).toHaveBeenCalledWith(expect.any(Error)); + }); + + it('does nothing when projects list is empty', async () => { + const emptyProjectsMutate = renderHookWithProviders( + () => useBulkMutateSelectedAgent({projects: []}), + {organization} + ); + const onSuccess = jest.fn(); + + act(() => { + emptyProjectsMutate.result.current('seer', {onSuccess}); + }); + + await waitFor(() => { + expect(onSuccess).toHaveBeenCalledTimes(1); + }); + }); + }); + + describe('useBulkMutateCreatePr', () => { + const project1 = ProjectFixture({slug: 'project-slug', id: '1'}); + const project2 = ProjectFixture({slug: 'project-slug-2', id: '2'}); + const projects = [project1, project2]; + + const seerPreference: ProjectSeerPreferences = { + repositories: [], + automated_run_stopping_point: 'code_changes', + automation_handoff: undefined, + }; + + const integrationPreference: ProjectSeerPreferences = { + repositories: [], + automated_run_stopping_point: 'code_changes', + automation_handoff: { + handoff_point: 'root_cause', + target: CodingAgentProvider.CURSOR_BACKGROUND_AGENT, + integration_id: 123, + auto_create_pr: false, + }, + }; + + function setupMocks( + p1Preference: ProjectSeerPreferences = seerPreference, + p2Preference: ProjectSeerPreferences = seerPreference + ) { + const perProjectPrefs = [p1Preference, p2Preference]; + const mocks = projects.map((p, i) => ({ + seerPreferencesGetRequest: MockApiClient.addMockResponse({ + url: `/projects/${organization.slug}/${p.slug}/seer/preferences/`, + method: 'GET', + body: { + preference: perProjectPrefs[i], + code_mapping_repos: [], + } satisfies SeerPreferencesResponse, + }), + seerPreferencesPostRequest: MockApiClient.addMockResponse({ + url: `/projects/${organization.slug}/${p.slug}/seer/preferences/`, + method: 'POST', + body: {}, + }), + })); + return { + seerPreferencesGetRequests: mocks.map(m => m.seerPreferencesGetRequest), + seerPreferencesPostRequests: mocks.map(m => m.seerPreferencesPostRequest), + }; + } + + function renderBulkMutateCreatePr() { + return renderHookWithProviders( + (props: {projects: typeof projects}) => { + const mutate = useBulkMutateCreatePr(props); + return {mutate}; + }, + { + initialProps: {projects}, + organization, + } + ); + } + + beforeEach(() => { + ProjectsStore.loadInitialData(projects); + }); + + it('sets automated_run_stopping_point for Seer projects when enabling Create PR', async () => { + const {seerPreferencesPostRequests} = setupMocks(); + const {result} = renderBulkMutateCreatePr(); + + act(() => { + result.current.mutate(true, {}); + }); + + await waitFor(() => { + expect(seerPreferencesPostRequests[0]).toHaveBeenCalledTimes(1); + }); + + projects.forEach((p, i) => { + expect(seerPreferencesPostRequests[i]).toHaveBeenCalledWith( + `/projects/${organization.slug}/${p.slug}/seer/preferences/`, + expect.objectContaining({ + method: 'POST', + data: expect.objectContaining({ + automated_run_stopping_point: 'open_pr', + automation_handoff: undefined, + }), + }) + ); + }); + }); + + it('sets automated_run_stopping_point for Seer projects when disabling Create PR', async () => { + const {seerPreferencesPostRequests} = setupMocks( + {...seerPreference, automated_run_stopping_point: 'open_pr'}, + {...seerPreference, automated_run_stopping_point: 'open_pr'} + ); + const {result} = renderBulkMutateCreatePr(); + + act(() => { + result.current.mutate(false, {}); + }); + + await waitFor(() => { + expect(seerPreferencesPostRequests[0]).toHaveBeenCalledTimes(1); + }); + + projects.forEach((p, i) => { + expect(seerPreferencesPostRequests[i]).toHaveBeenCalledWith( + `/projects/${organization.slug}/${p.slug}/seer/preferences/`, + expect.objectContaining({ + data: expect.objectContaining({ + automated_run_stopping_point: 'code_changes', + }), + }) + ); + }); + }); + + it('sets auto_create_pr in automation_handoff for external agent projects', async () => { + const {seerPreferencesPostRequests} = setupMocks( + integrationPreference, + integrationPreference + ); + const {result} = renderBulkMutateCreatePr(); + + act(() => { + result.current.mutate(true, {}); + }); + + await waitFor(() => { + expect(seerPreferencesPostRequests[0]).toHaveBeenCalledTimes(1); + }); + + projects.forEach((p, i) => { + expect(seerPreferencesPostRequests[i]).toHaveBeenCalledWith( + `/projects/${organization.slug}/${p.slug}/seer/preferences/`, + expect.objectContaining({ + data: expect.objectContaining({ + automation_handoff: expect.objectContaining({ + integration_id: 123, + auto_create_pr: true, + }), + }), + }) + ); + }); + }); + + it('disables auto_create_pr in automation_handoff for external agent projects', async () => { + const {seerPreferencesPostRequests} = setupMocks( + { + ...integrationPreference, + automation_handoff: { + ...integrationPreference.automation_handoff!, + auto_create_pr: true, + }, + }, + { + ...integrationPreference, + automation_handoff: { + ...integrationPreference.automation_handoff!, + auto_create_pr: true, + }, + } + ); + const {result} = renderBulkMutateCreatePr(); + + act(() => { + result.current.mutate(false, {}); + }); + + await waitFor(() => { + expect(seerPreferencesPostRequests[0]).toHaveBeenCalledTimes(1); + }); + + projects.forEach((_p, i) => { + expect(seerPreferencesPostRequests[i]).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + data: expect.objectContaining({ + automation_handoff: expect.objectContaining({ + auto_create_pr: false, + }), + }), + }) + ); + }); + }); + + it('handles mixed projects — Seer and external agent — correctly', async () => { + const {seerPreferencesPostRequests} = setupMocks( + seerPreference, + integrationPreference + ); + const {result} = renderBulkMutateCreatePr(); + + act(() => { + result.current.mutate(true, {}); + }); + + await waitFor(() => { + expect(seerPreferencesPostRequests[0]).toHaveBeenCalledTimes(1); + }); + + // project1 (Seer): uses stopping_point + expect(seerPreferencesPostRequests[0]).toHaveBeenCalledWith( + `/projects/${organization.slug}/${project1.slug}/seer/preferences/`, + expect.objectContaining({ + data: expect.objectContaining({ + automated_run_stopping_point: 'open_pr', + automation_handoff: undefined, + }), + }) + ); + // project2 (external agent): uses auto_create_pr in handoff + expect(seerPreferencesPostRequests[1]).toHaveBeenCalledWith( + `/projects/${organization.slug}/${project2.slug}/seer/preferences/`, + expect.objectContaining({ + data: expect.objectContaining({ + automation_handoff: expect.objectContaining({ + integration_id: 123, + auto_create_pr: true, + }), + }), + }) + ); + }); + + it('preserves existing repositories and other handoff fields', async () => { + const preferenceWithRepos: ProjectSeerPreferences = { + repositories: [ + {external_id: 'repo-1', name: 'my-repo', owner: 'my-org', provider: 'github'}, + ], + automated_run_stopping_point: 'code_changes', + automation_handoff: undefined, + }; + const {seerPreferencesPostRequests} = setupMocks( + preferenceWithRepos, + preferenceWithRepos + ); + const {result} = renderBulkMutateCreatePr(); + + act(() => { + result.current.mutate(true, {}); + }); + + await waitFor(() => { + expect(seerPreferencesPostRequests[0]).toHaveBeenCalledTimes(1); + }); + + projects.forEach((_p, i) => { + expect(seerPreferencesPostRequests[i]).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + data: expect.objectContaining({ + repositories: [ + { + external_id: 'repo-1', + name: 'my-repo', + owner: 'my-org', + provider: 'github', + }, + ], + }), + }) + ); + }); + }); + + it('calls onSuccess when all requests succeed', async () => { + setupMocks(); + const onSuccess = jest.fn(); + const {result} = renderBulkMutateCreatePr(); + + act(() => { + result.current.mutate(true, {onSuccess}); + }); + + await waitFor(() => { + expect(onSuccess).toHaveBeenCalledTimes(1); + }); + }); + + it('calls onError when any request fails', async () => { + projects.forEach(p => { + MockApiClient.addMockResponse({ + url: `/projects/${organization.slug}/${p.slug}/seer/preferences/`, + method: 'GET', + body: { + preference: seerPreference, + code_mapping_repos: [], + } satisfies SeerPreferencesResponse, + }); + MockApiClient.addMockResponse({ + url: `/projects/${organization.slug}/${p.slug}/seer/preferences/`, + method: 'POST', + statusCode: 500, + body: {}, + }); + }); + const onError = jest.fn(); + const {result} = renderBulkMutateCreatePr(); + + act(() => { + result.current.mutate(true, {onError}); + }); + + await waitFor(() => { + expect(onError).toHaveBeenCalledTimes(1); + }); + expect(onError).toHaveBeenCalledWith(expect.any(Error)); + }); + + it('does nothing when projects list is empty', async () => { + const emptyProjectsMutate = renderHookWithProviders( + () => useBulkMutateCreatePr({projects: []}), + {organization} + ); + const onSuccess = jest.fn(); + + act(() => { + emptyProjectsMutate.result.current(true, {onSuccess}); + }); + + await waitFor(() => { + expect(onSuccess).toHaveBeenCalledTimes(1); + }); + }); + }); + describe('useMutateCreatePr', () => { const basePreference: ProjectSeerPreferences = { repositories: [], diff --git a/static/app/views/settings/seer/seerAgentHooks.tsx b/static/app/views/settings/seer/seerAgentHooks.tsx index 4c19a50cf6410d..7aac636bce3226 100644 --- a/static/app/views/settings/seer/seerAgentHooks.tsx +++ b/static/app/views/settings/seer/seerAgentHooks.tsx @@ -1,9 +1,12 @@ import {useCallback, useMemo} from 'react'; +import {addErrorMessage} from 'sentry/actionCreators/indicator'; import { bulkAutofixAutomationSettingsInfiniteOptions, type AutofixAutomationSettings, } from 'sentry/components/events/autofix/preferences/hooks/useBulkAutofixAutomationSettings'; +import {makeProjectSeerPreferencesQueryKey} from 'sentry/components/events/autofix/preferences/hooks/useProjectSeerPreferences'; +import type {SeerPreferencesResponse} from 'sentry/components/events/autofix/preferences/hooks/useProjectSeerPreferences'; import { useFetchProjectSeerPreferences, useUpdateProjectSeerPreferences, @@ -14,8 +17,10 @@ import {type CodingAgentIntegration} from 'sentry/components/events/autofix/useA import {t} from 'sentry/locale'; import {ProjectsStore} from 'sentry/stores/projectsStore'; import type {Project} from 'sentry/types/project'; +import {processInChunks} from 'sentry/utils/array/procesInChunks'; import {useUpdateProject} from 'sentry/utils/project/useUpdateProject'; -import {useQueryClient} from 'sentry/utils/queryClient'; +import {fetchDataQuery, fetchMutation, useQueryClient} from 'sentry/utils/queryClient'; +import {RequestError} from 'sentry/utils/requestError/requestError'; import {useOrganization} from 'sentry/utils/useOrganization'; export function useAgentOptions({ @@ -95,11 +100,6 @@ export function useSelectedAgentFromBulkSettings({ ]); } -type MutateOptions = { - onError?: (error: Error) => void; - onSuccess?: () => void; -}; - function useApplyOptimisticUpdate({project}: {project: Project}) { const queryClient = useQueryClient(); const organization = useOrganization(); @@ -140,6 +140,11 @@ function useApplyOptimisticUpdate({project}: {project: Project}) { ); } +type MutateOptions = { + onError?: (error: Error) => void; + onSuccess?: () => void; +}; + export function useMutateSelectedAgent({project}: {project: Project}) { const {mutateAsync: updateProject} = useUpdateProject(project); const {mutateAsync: updateProjectSeerPreferences} = @@ -209,6 +214,159 @@ export function useMutateSelectedAgent({project}: {project: Project}) { ); } +export function useBulkMutateSelectedAgent({projects}: {projects: Project[]}) { + const organization = useOrganization(); + const queryClient = useQueryClient(); + const autofixSettingsQueryOptions = bulkAutofixAutomationSettingsInfiniteOptions({ + organization, + }); + + return useCallback( + async ( + integration: 'seer' | 'none' | CodingAgentIntegration, + {onSuccess, onError}: MutateOptions + ) => { + const results = await processInChunks({ + items: projects, + chunkSize: 15, + fn: async project => { + const [preferencesData] = await queryClient.fetchQuery({ + queryKey: makeProjectSeerPreferencesQueryKey(organization.slug, project.slug), + queryFn: fetchDataQuery, + staleTime: 0, + }); + const preference = preferencesData?.preference; + + const handoff: ProjectSeerPreferences['automation_handoff'] = + integration !== 'seer' && integration !== 'none' && integration + ? { + handoff_point: 'root_cause', + target: PROVIDER_TO_HANDOFF_TARGET[integration.provider]!, + integration_id: Number(integration.id), + auto_create_pr: preference?.automated_run_stopping_point === 'open_pr', + } + : undefined; + + return Promise.all([ + fetchMutation({ + method: 'PUT', + url: `/projects/${organization.slug}/${project.slug}/`, + data: {autofixAutomationTuning: integration === 'none' ? 'off' : 'medium'}, + }), + fetchMutation({ + method: 'POST', + url: `/projects/${organization.slug}/${project.slug}/seer/preferences/`, + data: { + repositories: preference?.repositories ?? [], + automated_run_stopping_point: preference?.automated_run_stopping_point, + automation_handoff: handoff, + }, + }), + ]); + }, + }); + + // Update store only for projects that succeeded + results.forEach((result, i) => { + if (result.status === 'fulfilled') { + ProjectsStore.onUpdateSuccess({ + id: projects[i]!.id, + autofixAutomationTuning: integration === 'none' ? 'off' : 'medium', + }); + } + }); + + // Always invalidate to sync cache with whatever the server actually saved + queryClient.invalidateQueries({ + queryKey: autofixSettingsQueryOptions.queryKey, + }); + + const failures = results.filter(r => r.status === 'rejected'); + if (failures.length === 0) { + onSuccess?.(); + } else { + const has429 = failures.some( + r => r.reason instanceof RequestError && r.reason.status === 429 + ); + if (has429) { + addErrorMessage( + t('Too many requests. Please wait a moment before trying again.') + ); + } else { + onError?.(new Error('Failed to update agent setting')); + } + } + }, + [projects, organization, queryClient, autofixSettingsQueryOptions.queryKey] + ); +} + +export function useBulkMutateCreatePr({projects}: {projects: Project[]}) { + const organization = useOrganization(); + const queryClient = useQueryClient(); + const autofixSettingsQueryOptions = bulkAutofixAutomationSettingsInfiniteOptions({ + organization, + }); + + return useCallback( + async (value: boolean, {onSuccess, onError}: MutateOptions) => { + const results = await processInChunks({ + items: projects, + chunkSize: 10, + fn: async project => { + const [preferencesData] = await queryClient.fetchQuery({ + queryKey: makeProjectSeerPreferencesQueryKey(organization.slug, project.slug), + queryFn: fetchDataQuery, + staleTime: 0, + }); + const preference = preferencesData?.preference; + + return fetchMutation({ + method: 'POST', + url: `/projects/${organization.slug}/${project.slug}/seer/preferences/`, + data: preference?.automation_handoff?.integration_id + ? { + repositories: preference?.repositories ?? [], + automated_run_stopping_point: preference?.automated_run_stopping_point, + automation_handoff: { + ...preference.automation_handoff, + auto_create_pr: value, + }, + } + : { + repositories: preference?.repositories ?? [], + automated_run_stopping_point: value ? 'open_pr' : 'code_changes', + automation_handoff: preference?.automation_handoff, + }, + }); + }, + }); + + // Always invalidate to sync cache with whatever the server actually saved + queryClient.invalidateQueries({ + queryKey: autofixSettingsQueryOptions.queryKey, + }); + + const failures = results.filter(r => r.status === 'rejected'); + if (failures.length === 0) { + onSuccess?.(); + } else { + const has429 = failures.some( + r => r.reason instanceof RequestError && r.reason.status === 429 + ); + if (has429) { + addErrorMessage( + t('Too many requests. Please wait a moment before trying again.') + ); + } else { + onError?.(new Error('Failed to update PR setting')); + } + } + }, + [projects, organization, queryClient, autofixSettingsQueryOptions.queryKey] + ); +} + export function useMutateCreatePr({project}: {project: Project}) { const {mutateAsync: updateProjectSeerPreferences} = useUpdateProjectSeerPreferences(project); diff --git a/static/gsAdmin/components/addBillingMetricUsage.tsx b/static/gsAdmin/components/addBillingMetricUsage.tsx deleted file mode 100644 index 4fbbb44d0a424b..00000000000000 --- a/static/gsAdmin/components/addBillingMetricUsage.tsx +++ /dev/null @@ -1,260 +0,0 @@ -import {Fragment, useState} from 'react'; - -import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator'; -import type {ModalRenderProps} from 'sentry/actionCreators/modal'; -import {openModal} from 'sentry/actionCreators/modal'; -import {InputField} from 'sentry/components/forms/fields/inputField'; -import {NumberField} from 'sentry/components/forms/fields/numberField'; -import {SelectField} from 'sentry/components/forms/fields/selectField'; -import {Form} from 'sentry/components/forms/form'; -import {LoadingIndicator} from 'sentry/components/loadingIndicator'; -import type {Organization} from 'sentry/types/organization'; -import type {Project} from 'sentry/types/project'; -import {getApiUrl} from 'sentry/utils/api/getApiUrl'; -import {useApiQuery} from 'sentry/utils/queryClient'; -import {useApi} from 'sentry/utils/useApi'; - -function getDateString(date: Date): string { - // returns date in YYYY-MM-DD format - return date.toISOString().split('T')[0]!; -} - -type CategoryInfo = { - api_name: string; - billed_category: number; - display_name: string; - name: string; - order: number; - product_name: string; - singular: string; - tally_type: number; -}; - -type BillingConfig = { - category_info: Record; - outcomes: Record; - reason_codes: Record; -}; - -type AdminRecordUsageRequest = { - data_category: number; - // YYYY-MM-DD format - date: string; - outcome: number; - project_id: number; - quantity: number; - reason_code: number; -}; - -type Props = { - onSuccess: () => void; - organization: Organization; -}; - -type ModalProps = Props & ModalRenderProps; - -function AddBillingMetricUsageModal({ - onSuccess, - organization, - closeModal, - Header, - Body, -}: ModalProps) { - const api = useApi(); - const [projectID, setProjectID] = useState(null); - const [dataCategory, setDataCategory] = useState(null); - const [quantity, setQuantity] = useState(0); - // set default outcome to 0 (ACCEPTED) - const [outcome, setOutcome] = useState(0); - // set default reason code to 0 (DEFAULT) - const [reason_code, setReasonCode] = useState(0); - const [date, setDate] = useState(new Date()); - const orgSlug = organization.slug; - - const {data: billingConfig = null, isPending: isLoadingBillingConfig} = - useApiQuery([getApiUrl('/billing-config/')], { - staleTime: Infinity, - }); - - const {data: projects = [], isPending: isLoadingProjects} = useApiQuery( - [ - getApiUrl(`/organizations/$organizationIdOrSlug/projects/`, { - path: {organizationIdOrSlug: orgSlug}, - }), - { - query: {all_projects: '1'}, - }, - ], - { - staleTime: Infinity, - } - ); - - if (isLoadingBillingConfig || isLoadingProjects || !billingConfig) { - return ( - -
Add Billing Metric Usage
- - - - - ); - } - - const dataCategoryChoices = Object.entries(billingConfig.category_info).map( - ([key, value]) => { - const billingMetric = Number(key); - return [billingMetric, `${value.display_name} (${billingMetric})`] as [ - number, - string, - ]; - } - ); - - const outcomeChoices = Object.entries(billingConfig.outcomes).map(([key, value]) => { - const outcomeID = Number(key); - return [outcomeID, `${value} (${outcomeID})`] as [number, string]; - }); - - const reasonCodeChoices = Object.entries(billingConfig.reason_codes).map( - ([key, value]) => { - const reasonCodeID = Number(key); - return [reasonCodeID, `${value} (${reasonCodeID})`] as [number, string]; - } - ); - - const onSubmit = () => { - if (projectID === null || dataCategory === null || quantity <= 0) { - return; - } - - const data: AdminRecordUsageRequest = { - project_id: projectID, - quantity, - data_category: dataCategory, - outcome, - reason_code, - date: getDateString(date), - }; - - api.request(`/customers/${orgSlug}/record-usage/`, { - method: 'POST', - data, - success: () => { - addSuccessMessage('Created billing metric usage.'); - closeModal(); - onSuccess(); - }, - error: () => { - addErrorMessage('Unable to create billing metric usage.'); - }, - }); - }; - - return ( - -
Add Billing Metric Usage
- -
Create and add mock billing metric usage for testing purposes.
-
-
- { - setProjectID(value); - }} - choices={projects.map(project => [ - Number(project.id), - `${project.slug} - ${project.name}`, - ])} - required - /> - { - setDataCategory(value); - }} - choices={dataCategoryChoices} - required - /> - Enter amount of usage for the chosen data category. - } - name="quantity" - value={quantity} - defaultValue={quantity} - min={0} - onChange={(value: number) => { - setQuantity(value); - }} - required - /> - { - setOutcome(value); - }} - choices={outcomeChoices} - required - /> - { - setReasonCode(value); - }} - choices={reasonCodeChoices} - required - /> - { - setDate(new Date(value)); - }} - /> - - -
- ); -} - -type Options = Pick; - -export const addBillingMetricUsage = (opts: Options) => - openModal(deps => , { - closeEvents: 'escape-key', - }); diff --git a/static/gsAdmin/components/addToStartupProgramAction.spec.tsx b/static/gsAdmin/components/addToStartupProgramAction.spec.tsx index 8295bc9cd9c83e..df0908a33b6096 100644 --- a/static/gsAdmin/components/addToStartupProgramAction.spec.tsx +++ b/static/gsAdmin/components/addToStartupProgramAction.spec.tsx @@ -64,7 +64,7 @@ describe('AddToStartupProgramAction', () => { expect(await screen.findByRole('spinbutton', {name: 'Credit Amount'})).toHaveValue( 5000 ); - expect(screen.getByRole('textbox', {name: 'Notes'})).toHaveValue('sentryforstartups'); + expect(screen.getByText('sentryforstartups')).toBeInTheDocument(); }); it('can submit with default values', async () => { @@ -97,7 +97,69 @@ describe('AddToStartupProgramAction', () => { expect(onSuccess).toHaveBeenCalled(); }); - it('can submit with custom values', async () => { + it('can submit with a different option selected', async () => { + const updateMock = MockApiClient.addMockResponse({ + url: `/_admin/customers/${organization.slug}/balance-changes/`, + method: 'POST', + body: {}, + }); + + triggerAddToStartupProgramModal(modalProps); + + const {waitForModalToHide} = renderGlobalModal(); + + await userEvent.click(await screen.findByText('sentryforstartups')); + await userEvent.click(screen.getByText('ycombinator')); + + await userEvent.click(screen.getByRole('button', {name: 'Submit'})); + + await waitForModalToHide(); + + await waitFor(() => { + expect(updateMock).toHaveBeenCalledWith( + `/_admin/customers/${organization.slug}/balance-changes/`, + expect.objectContaining({ + method: 'POST', + data: { + creditAmount: 500000, + ticketUrl: '', + notes: 'ycombinator', + }, + }) + ); + }); + }); + + it('shows custom notes field when "Enter custom notes" is selected', async () => { + triggerAddToStartupProgramModal(modalProps); + + renderGlobalModal(); + + expect(screen.queryByRole('textbox', {name: 'Custom Notes'})).not.toBeInTheDocument(); + + await userEvent.click(await screen.findByText('sentryforstartups')); + await userEvent.click(screen.getByText('Enter custom notes')); + + expect(screen.getByRole('textbox', {name: 'Custom Notes'})).toBeInTheDocument(); + }); + + it('hides custom notes field when switching back to a preset option', async () => { + triggerAddToStartupProgramModal(modalProps); + + renderGlobalModal(); + + // Select "Enter custom notes" + await userEvent.click(await screen.findByText('sentryforstartups')); + await userEvent.click(screen.getByText('Enter custom notes')); + expect(screen.getByRole('textbox', {name: 'Custom Notes'})).toBeInTheDocument(); + + // Switch back to a preset option + await userEvent.click(screen.getByText('Enter custom notes')); + await userEvent.click(screen.getByText('a16z')); + expect(screen.queryByRole('textbox', {name: 'Custom Notes'})).not.toBeInTheDocument(); + }); + + it('can submit with custom notes', async () => { const updateMock = MockApiClient.addMockResponse({ url: `/_admin/customers/${organization.slug}/balance-changes/`, method: 'POST', @@ -115,9 +177,13 @@ describe('AddToStartupProgramAction', () => { await userEvent.type(screen.getByRole('textbox', {name: 'Ticket URL'}), url); - const notesInput = screen.getByRole('textbox', {name: 'Notes'}); - await userEvent.clear(notesInput); - await userEvent.type(notesInput, 'custom note'); + await userEvent.click(screen.getByText('sentryforstartups')); + await userEvent.click(screen.getByText('Enter custom notes')); + + await userEvent.type( + screen.getByRole('textbox', {name: 'Custom Notes'}), + 'custom note' + ); await userEvent.click(screen.getByRole('button', {name: 'Submit'})); @@ -156,7 +222,6 @@ describe('AddToStartupProgramAction', () => { expect(submitButton).toBeDisabled(); expect(screen.getByRole('spinbutton', {name: 'Credit Amount'})).toBeDisabled(); expect(screen.getByRole('textbox', {name: 'Ticket URL'})).toBeDisabled(); - expect(screen.getByRole('textbox', {name: 'Notes'})).toBeDisabled(); await waitForModalToHide(); }); @@ -205,7 +270,6 @@ describe('AddToStartupProgramAction', () => { {timeout: 5_000} ); expect(screen.getByRole('textbox', {name: 'Ticket URL'})).toBeEnabled(); - expect(screen.getByRole('textbox', {name: 'Notes'})).toBeEnabled(); expect(screen.getByRole('button', {name: /submit/i})).toBeEnabled(); }, 25_000); diff --git a/static/gsAdmin/components/addToStartupProgramAction.tsx b/static/gsAdmin/components/addToStartupProgramAction.tsx index 86b083cbb1d8b1..f6ac174bf1e447 100644 --- a/static/gsAdmin/components/addToStartupProgramAction.tsx +++ b/static/gsAdmin/components/addToStartupProgramAction.tsx @@ -1,4 +1,4 @@ -import {Fragment} from 'react'; +import {Fragment, useState} from 'react'; import {Flex} from '@sentry/scraps/layout'; import {Heading, Text} from '@sentry/scraps/text'; @@ -8,6 +8,7 @@ import type {ModalRenderProps} from 'sentry/actionCreators/modal'; import {openModal} from 'sentry/actionCreators/modal'; import {InputField} from 'sentry/components/forms/fields/inputField'; import {NumberField} from 'sentry/components/forms/fields/numberField'; +import {SelectField} from 'sentry/components/forms/fields/selectField'; import {TextField} from 'sentry/components/forms/fields/textField'; import {Form, type FormProps} from 'sentry/components/forms/form'; import {fetchMutation, useMutation} from 'sentry/utils/queryClient'; @@ -16,6 +17,20 @@ import type {RequestError} from 'sentry/utils/requestError/requestError'; import type {Subscription} from 'getsentry/types'; import {formatBalance} from 'getsentry/utils/billing'; +const STARTUP_PROGRAM_OPTIONS = [ + {value: 'ycombinator', label: 'ycombinator'}, + {value: 'sentryforstartups', label: 'sentryforstartups'}, + {value: 'a16z', label: 'a16z'}, + {value: 'accelatoms', label: 'accelatoms'}, + {value: 'accelfam', label: 'accelfam'}, + {value: 'renderstack', label: 'renderstack'}, + {value: 'finpack', label: 'finpack'}, + {value: 'betaworks', label: 'betaworks'}, + {value: 'alchemist', label: 'alchemist'}, + {value: 'antler', label: 'antler'}, + {value: 'other', label: 'Enter custom notes'}, +]; + function coerceValue(value: number) { if (isNaN(value)) { return undefined; @@ -46,6 +61,8 @@ function AddToStartupProgramModal({ Header, Body, }: AddToStartupProgramModalProps) { + const [showCustomNotes, setShowCustomNotes] = useState(false); + const {mutate, isPending} = useMutation< Record, RequestError, @@ -82,7 +99,13 @@ function AddToStartupProgramModal({ const creditAmountInput = Number(data.creditAmount); const creditAmount = coerceValue(creditAmountInput); const ticketUrl = typeof data.ticketUrl === 'string' ? data.ticketUrl : ''; - const notes = typeof data.notes === 'string' ? data.notes : ''; + const rawNotes = typeof data.notes === 'string' ? data.notes : ''; + const notes = + rawNotes === 'other' + ? typeof data.customNotes === 'string' + ? data.customNotes + : '' + : rawNotes; if (!creditAmount || isPending) { return; @@ -139,14 +162,27 @@ function AddToStartupProgramModal({ stacked disabled={isPending} /> - { + setShowCustomNotes(value === 'other'); + }} /> + {showCustomNotes && ( + + )} diff --git a/static/gsAdmin/views/customerDetails.tsx b/static/gsAdmin/views/customerDetails.tsx index 29adb08e5e6a47..0396f5bb8a8519 100644 --- a/static/gsAdmin/views/customerDetails.tsx +++ b/static/gsAdmin/views/customerDetails.tsx @@ -34,7 +34,6 @@ import {useNavigate} from 'sentry/utils/useNavigate'; import {useParams} from 'sentry/utils/useParams'; import {OrganizationContext} from 'sentry/views/organizationContext'; -import {addBillingMetricUsage} from 'admin/components/addBillingMetricUsage'; import {addGiftBudgetAction} from 'admin/components/addGiftBudgetAction'; import {AddGiftEventsAction} from 'admin/components/addGiftEventsAction'; import {triggerAddToStartupProgramModal} from 'admin/components/addToStartupProgramAction'; @@ -814,18 +813,6 @@ export function CustomerDetails() { }); }, }, - { - key: 'addBillingMetricUsage', - name: 'Add Billing Metric Usage', - help: 'Create and add Billing Metric Usage.', - skipConfirmModal: true, - visible: hasAdminTestFeatures, - onAction: () => - addBillingMetricUsage({ - onSuccess: reloadData, - organization, - }), - }, { key: 'deleteBillingMetricHistory', name: 'Delete Billing Metric History', diff --git a/static/gsApp/hooks/githubInstallationSelectInstall.spec.tsx b/static/gsApp/hooks/githubInstallationSelectInstall.spec.tsx deleted file mode 100644 index 8b69fedc9427ab..00000000000000 --- a/static/gsApp/hooks/githubInstallationSelectInstall.spec.tsx +++ /dev/null @@ -1,106 +0,0 @@ -import {OrganizationFixture} from 'sentry-fixture/organization'; - -import {SubscriptionFixture} from 'getsentry-test/fixtures/subscription'; -import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; - -import {trackAnalytics} from 'sentry/utils/analytics'; - -import {openUpsellModal} from 'getsentry/actionCreators/modal'; - -import GithubInstallationSelectInstallButton from './githubInstallationSelectInstall'; - -// Mock the functions -jest.mock('getsentry/actionCreators/modal', () => ({ - openUpsellModal: jest.fn(), -})); - -jest.mock('sentry/utils/analytics', () => ({ - trackAnalytics: jest.fn(), -})); - -describe('GithubInstallationSelectInstallButton', () => { - const organization = OrganizationFixture(); - const subscription = SubscriptionFixture({ - organization, - }); - - const defaultProps = { - handleSubmit: jest.fn(), - hasSCMMultiOrg: true, - installationID: '-1', - isSaving: false, - subscription, - }; - - beforeEach(() => { - jest.clearAllMocks(); - MockApiClient.clearMockResponses(); - }); - - it('renders Install button when installationID is -1', async () => { - MockApiClient.addMockResponse({ - url: '/customers/org-slug/', - method: 'GET', - body: subscription, - }); - - render(); - expect(await screen.findByText('Install')).toBeInTheDocument(); - }); - - it('renders Upgrade button when has_scm_multi_org is false and installationID is not -1', () => { - render( - - ); - expect(screen.getByText('Upgrade')).toBeInTheDocument(); - }); - - it('renders Install button when has_scm_multi_org is true and installationID is not -1', () => { - render( - - ); - expect(screen.getByText('Install')).toBeInTheDocument(); - }); - - it('calls handleSubmit when clicked', () => { - render(); - screen.getByRole('button').click(); - expect(defaultProps.handleSubmit).toHaveBeenCalled(); - }); - - it('opens upsell modal and tracks analytics when Upgrade button is clicked', async () => { - MockApiClient.addMockResponse({ - url: '/customers/org-slug/', - method: 'GET', - body: subscription, - }); - - render( - - ); - - await userEvent.click(screen.getByRole('button', {name: 'Upgrade'})); - - expect(trackAnalytics).toHaveBeenCalledWith('github.multi_org.upsell', { - organization, - subscriptionTier: subscription.planTier, - }); - - expect(openUpsellModal).toHaveBeenCalledWith({ - source: 'github.multi_org', - organization, - }); - }); -}); diff --git a/static/gsApp/hooks/githubInstallationSelectInstall.tsx b/static/gsApp/hooks/githubInstallationSelectInstall.tsx deleted file mode 100644 index ec2791139dde6d..00000000000000 --- a/static/gsApp/hooks/githubInstallationSelectInstall.tsx +++ /dev/null @@ -1,76 +0,0 @@ -import {Fragment, useCallback, useRef} from 'react'; -import styled from '@emotion/styled'; - -import {Button} from '@sentry/scraps/button'; -import type {SelectKey} from '@sentry/scraps/compactSelect'; - -import {GlobalModal} from 'sentry/components/globalModal'; -import {IconLightning} from 'sentry/icons'; -import {t} from 'sentry/locale'; -import {trackAnalytics} from 'sentry/utils/analytics'; -import {useOrganization} from 'sentry/utils/useOrganization'; - -import {openUpsellModal} from 'getsentry/actionCreators/modal'; -import {withSubscription} from 'getsentry/components/withSubscription'; -import type {Subscription} from 'getsentry/types'; - -type Props = { - handleSubmit: (e: React.MouseEvent) => void; - hasSCMMultiOrg: boolean; - installationID: SelectKey; - isSaving: boolean; - subscription: Subscription; -}; - -function GithubInstallationSelectInstallButton({ - hasSCMMultiOrg, - installationID, - subscription, - handleSubmit, - isSaving, -}: Props) { - const organization = useOrganization(); - const source = 'github.multi_org'; - const mainContainerRef = useRef(null); - const handleModalClose = useCallback(() => { - mainContainerRef.current?.focus?.(); - }, []); - - if (installationID === '-1' || hasSCMMultiOrg) { - return ( - - {t('Install')} - - ); - } - - return ( - - - } - priority="primary" - onClick={() => { - trackAnalytics(`${source}.upsell`, { - organization, - subscriptionTier: subscription.planTier, - }); - openUpsellModal({source, organization}); - }} - disabled={isSaving || !installationID} - > - {t('Upgrade')} - - - ); -} - -const StyledButton = styled(Button)` - margin-left: ${p => p.theme.space.sm}; - &:not(:disabled) { - background-color: #6c5fc7; - color: #fff; - } -`; - -export default withSubscription(GithubInstallationSelectInstallButton); diff --git a/static/gsApp/registerHooks.tsx b/static/gsApp/registerHooks.tsx index 0dc7f12d49faac..d28046b05e5ee2 100644 --- a/static/gsApp/registerHooks.tsx +++ b/static/gsApp/registerHooks.tsx @@ -52,7 +52,6 @@ import DisabledMemberTooltip from 'getsentry/hooks/disabledMemberTooltip'; import DisabledMemberView from 'getsentry/hooks/disabledMemberView'; import {FirstPartyIntegrationAdditionalCTA} from 'getsentry/hooks/firstPartyIntegrationAdditionalCTA'; import {FirstPartyIntegrationAlertHook} from 'getsentry/hooks/firstPartyIntegrationAlertHook'; -import GithubInstallationSelectInstallButton from 'getsentry/hooks/githubInstallationSelectInstall'; import {handleGuideUpdate} from 'getsentry/hooks/handleGuideUpdate'; import {handleMonitorCreated} from 'getsentry/hooks/handleMonitorCreated'; import {hookIntegrationFeatures} from 'getsentry/hooks/integrationFeatures'; @@ -260,7 +259,6 @@ const GETSENTRY_HOOKS: Partial = { 'component:data-consent-priority-learn-more': () => DataConsentPriorityLearnMore, 'component:data-consent-org-creation-checkbox': () => DataConsentOrgCreationCheckbox, 'component:organization-membership-settings': () => OrganizationMembershipSettingsForm, - 'component:scm-multi-org-install-button': () => GithubInstallationSelectInstallButton, 'component:metric-alert-quota-message': MetricAlertQuotaMessage, 'component:metric-alert-quota-icon': MetricAlertQuotaIcon, diff --git a/static/gsApp/views/seerAutomation/components/repoTable/seerRepoTable.tsx b/static/gsApp/views/seerAutomation/components/repoTable/seerRepoTable.tsx index 87ea64f66ada6d..f5faa68c1237c0 100644 --- a/static/gsApp/views/seerAutomation/components/repoTable/seerRepoTable.tsx +++ b/static/gsApp/views/seerAutomation/components/repoTable/seerRepoTable.tsx @@ -19,7 +19,6 @@ import { import {LoadingError} from 'sentry/components/loadingError'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {Panel} from 'sentry/components/panels/panel'; -import {ScmRepoTreeModal} from 'sentry/components/repositories/scmRepoTreeModal'; import {useBulkUpdateRepositorySettings} from 'sentry/components/repositories/useBulkUpdateRepositorySettings'; import {getRepositoryWithSettingsQueryKey} from 'sentry/components/repositories/useRepositoryWithSettings'; import {IconAdd} from 'sentry/icons'; @@ -161,7 +160,10 @@ export function SeerRepoTable() {