diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 891c0ad90ce9..408881367d3a 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -435,6 +435,7 @@ tests/sentry/api/endpoints/test_organization_attribute_mappings.py @get ## Frontend /static/app/components/analyticsArea.spec.tsx @getsentry/app-frontend /static/app/components/analyticsArea.tsx @getsentry/app-frontend +/static/app/components/loading/ @getsentry/app-frontend /static/app/components/events/interfaces/ @getsentry/app-frontend /static/app/components/forms/ @getsentry/app-frontend /static/app/locale.tsx @getsentry/app-frontend diff --git a/.github/workflows/label-pullrequest.yml b/.github/workflows/label-pullrequest.yml index 48cfe4f909e6..22c7cd534255 100644 --- a/.github/workflows/label-pullrequest.yml +++ b/.github/workflows/label-pullrequest.yml @@ -24,13 +24,13 @@ jobs: filters: .github/file-filters.yml - name: Add frontend label - uses: getsentry/action-add-labels@54d0cba498c1eaf8bd34985d715504d1b6e2935f + uses: getsentry/action-add-labels@ca568508c6e91387909cb3661e4cd965aa2f3a89 if: steps.changes.outputs.frontend_all == 'true' with: labels: 'Scope: Frontend' - name: Add backend label - uses: getsentry/action-add-labels@54d0cba498c1eaf8bd34985d715504d1b6e2935f + uses: getsentry/action-add-labels@ca568508c6e91387909cb3661e4cd965aa2f3a89 if: steps.changes.outputs.backend_src == 'true' with: labels: 'Scope: Backend' diff --git a/pyproject.toml b/pyproject.toml index 46b1f3728e61..36d70464c304 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -87,7 +87,7 @@ dependencies = [ "sentry-forked-email-reply-parser>=0.5.12.post1", "sentry-kafka-schemas>=2.1.27", "sentry-ophio>=1.1.3", - "sentry-protos>=0.8.8", + "sentry-protos>=0.8.10", "sentry-redis-tools>=0.5.0", "sentry-relay>=0.9.25", "sentry-sdk[http2]>=2.47.0", @@ -683,6 +683,7 @@ module = [ "sentry.tasks.codeowners.*", "sentry.tasks.commit_context", "sentry.tasks.on_demand_metrics", + "sentry.tasks.post_process", "sentry.tasks.reprocessing2", "sentry.tasks.seer.delete_seer_grouping_records", "sentry.tasks.store", diff --git a/src/sentry/billing/platform/services/category_mapping.py b/src/sentry/billing/platform/services/category_mapping.py new file mode 100644 index 000000000000..5ca1ead0c9d2 --- /dev/null +++ b/src/sentry/billing/platform/services/category_mapping.py @@ -0,0 +1,67 @@ +from __future__ import annotations + +from sentry_protos.billing.v1.data_category_pb2 import DataCategory as ProtoDataCategory + +from sentry.constants import DataCategory +from sentry.utils import metrics + +SENTRY_TO_PROTO_CATEGORY: dict[int, int] = { + int(DataCategory.ERROR): ProtoDataCategory.DATA_CATEGORY_ERROR, + int(DataCategory.TRANSACTION): ProtoDataCategory.DATA_CATEGORY_TRANSACTION, + int(DataCategory.ATTACHMENT): ProtoDataCategory.DATA_CATEGORY_ATTACHMENT, + int(DataCategory.PROFILE): ProtoDataCategory.DATA_CATEGORY_PROFILE, + int(DataCategory.REPLAY): ProtoDataCategory.DATA_CATEGORY_REPLAY, + int(DataCategory.MONITOR): ProtoDataCategory.DATA_CATEGORY_MONITOR, + int(DataCategory.SPAN): ProtoDataCategory.DATA_CATEGORY_SPAN, + int(DataCategory.USER_REPORT_V2): ProtoDataCategory.DATA_CATEGORY_USER_REPORT_V2, + int(DataCategory.PROFILE_DURATION): ProtoDataCategory.DATA_CATEGORY_PROFILE_DURATION, + int(DataCategory.LOG_BYTE): ProtoDataCategory.DATA_CATEGORY_LOG_BYTE, + int(DataCategory.PROFILE_DURATION_UI): ProtoDataCategory.DATA_CATEGORY_PROFILE_DURATION_UI, + int(DataCategory.SEER_AUTOFIX): ProtoDataCategory.DATA_CATEGORY_SEER_AUTOFIX, + int(DataCategory.SEER_SCANNER): ProtoDataCategory.DATA_CATEGORY_SEER_SCANNER, + int(DataCategory.SIZE_ANALYSIS): ProtoDataCategory.DATA_CATEGORY_SIZE_ANALYSIS, + int(DataCategory.INSTALLABLE_BUILD): ProtoDataCategory.DATA_CATEGORY_INSTALLABLE_BUILD, + int(DataCategory.TRACE_METRIC): ProtoDataCategory.DATA_CATEGORY_TRACE_METRIC, + int(DataCategory.DEFAULT): ProtoDataCategory.DATA_CATEGORY_DEFAULT, + int(DataCategory.SECURITY): ProtoDataCategory.DATA_CATEGORY_SECURITY, + int(DataCategory.PROFILE_CHUNK): ProtoDataCategory.DATA_CATEGORY_PROFILE_CHUNK, + int(DataCategory.PROFILE_CHUNK_UI): ProtoDataCategory.DATA_CATEGORY_PROFILE_CHUNK_UI, +} + + +PROTO_TO_SENTRY_CATEGORY: dict[int, int] = {v: k for k, v in SENTRY_TO_PROTO_CATEGORY.items()} + + +def proto_to_sentry_category(proto_category: int) -> int: + """Convert a proto DataCategory to its Sentry equivalent. + + For categories with a known mapping, returns the sentry int value. + For unmapped categories, passes through the original int value and + emits a metric so we can track how often this happens. + """ + result = PROTO_TO_SENTRY_CATEGORY.get(proto_category) + if result is None: + metrics.incr( + "billing.proto_category_mapping.unmapped_reverse", + tags={"proto_category": str(proto_category)}, + ) + return proto_category + return result + + +def sentry_to_proto_category(category: int | DataCategory) -> ProtoDataCategory.ValueType: + """Convert a Sentry DataCategory to its proto equivalent. + + For categories with a known mapping, returns the proto enum value. + For unmapped categories, passes through the original int value and + emits a metric so we can track how often this happens. + """ + cat_int = int(category) + result = SENTRY_TO_PROTO_CATEGORY.get(cat_int) + if result is None: + metrics.incr( + "billing.proto_category_mapping.unmapped", + tags={"sentry_category": str(cat_int)}, + ) + return ProtoDataCategory.ValueType(cat_int) + return ProtoDataCategory.ValueType(result) diff --git a/src/sentry/billing/platform/services/usage/_category_mapping.py b/src/sentry/billing/platform/services/usage/_category_mapping.py deleted file mode 100644 index 8bbe76019934..000000000000 --- a/src/sentry/billing/platform/services/usage/_category_mapping.py +++ /dev/null @@ -1,50 +0,0 @@ -from __future__ import annotations - -import logging - -from sentry_protos.billing.v1.data_category_pb2 import DataCategory as ProtoDataCategory - -from sentry.constants import DataCategory -from sentry.utils import metrics - -logger = logging.getLogger(__name__) - -# Proto DataCategory enum uses different int values from Relay/Sentry DataCategory. -# e.g., Proto ATTACHMENT=3 vs Relay ATTACHMENT=4. -# ClickHouse stores Relay ints. The proto request carries proto ints. -# This mapping converts between the two. -PROTO_TO_RELAY_CATEGORY: dict[int, int] = { - ProtoDataCategory.DATA_CATEGORY_ERROR: int(DataCategory.ERROR), - ProtoDataCategory.DATA_CATEGORY_TRANSACTION: int(DataCategory.TRANSACTION), - ProtoDataCategory.DATA_CATEGORY_ATTACHMENT: int(DataCategory.ATTACHMENT), - ProtoDataCategory.DATA_CATEGORY_PROFILE: int(DataCategory.PROFILE), - ProtoDataCategory.DATA_CATEGORY_REPLAY: int(DataCategory.REPLAY), - ProtoDataCategory.DATA_CATEGORY_MONITOR: int(DataCategory.MONITOR), - ProtoDataCategory.DATA_CATEGORY_SPAN: int(DataCategory.SPAN), - ProtoDataCategory.DATA_CATEGORY_USER_REPORT_V2: int(DataCategory.USER_REPORT_V2), - ProtoDataCategory.DATA_CATEGORY_PROFILE_DURATION: int(DataCategory.PROFILE_DURATION), - ProtoDataCategory.DATA_CATEGORY_LOG_BYTE: int(DataCategory.LOG_BYTE), - ProtoDataCategory.DATA_CATEGORY_PROFILE_DURATION_UI: int(DataCategory.PROFILE_DURATION_UI), - ProtoDataCategory.DATA_CATEGORY_SEER_AUTOFIX: int(DataCategory.SEER_AUTOFIX), - ProtoDataCategory.DATA_CATEGORY_SEER_SCANNER: int(DataCategory.SEER_SCANNER), - ProtoDataCategory.DATA_CATEGORY_SIZE_ANALYSIS: int(DataCategory.SIZE_ANALYSIS), - ProtoDataCategory.DATA_CATEGORY_INSTALLABLE_BUILD: int(DataCategory.INSTALLABLE_BUILD), - ProtoDataCategory.DATA_CATEGORY_TRACE_METRIC: int(DataCategory.TRACE_METRIC), - ProtoDataCategory.DATA_CATEGORY_DEFAULT: int(DataCategory.DEFAULT), - ProtoDataCategory.DATA_CATEGORY_SECURITY: int(DataCategory.SECURITY), - ProtoDataCategory.DATA_CATEGORY_PROFILE_CHUNK: int(DataCategory.PROFILE_CHUNK), - ProtoDataCategory.DATA_CATEGORY_PROFILE_CHUNK_UI: int(DataCategory.PROFILE_CHUNK_UI), -} - - -def proto_to_relay_category(proto_category: int) -> int: - """Convert a proto DataCategory int to the Relay/Sentry int used in ClickHouse.""" - result = PROTO_TO_RELAY_CATEGORY.get(proto_category) - if result is None: - metrics.incr( - "billing.proto_category_mapping.unmapped", - tags={"proto_category": str(proto_category)}, - sample_rate=1.0, - ) - return proto_category - return result diff --git a/src/sentry/billing/platform/services/usage/_outcomes_query.py b/src/sentry/billing/platform/services/usage/_outcomes_query.py index 5e16ef64bab1..8efbec8be284 100644 --- a/src/sentry/billing/platform/services/usage/_outcomes_query.py +++ b/src/sentry/billing/platform/services/usage/_outcomes_query.py @@ -28,7 +28,7 @@ ) from snuba_sdk.orderby import Direction -from sentry.billing.platform.services.usage._category_mapping import proto_to_relay_category +from sentry.billing.platform.services.category_mapping import proto_to_sentry_category from sentry.snuba.referrer import Referrer from sentry.utils import metrics from sentry.utils.outcomes import Outcome @@ -58,7 +58,7 @@ def query_outcomes_usage(request: GetUsageRequest) -> GetUsageResponse: end = _timestamp_to_datetime(request.end) + timedelta(days=1) # Proto categories use different int values from Relay/ClickHouse # (e.g., proto ATTACHMENT=3 vs Relay ATTACHMENT=4). Convert before querying. - categories = [proto_to_relay_category(c) for c in request.categories] + categories = [proto_to_sentry_category(c) for c in request.categories] snuba_request = _build_query(org_id, start, end, categories, total_outcomes=_BILLABLE_OUTCOMES) result = raw_snql_query(snuba_request, referrer=_REFERRER) diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index 37584bd50d46..12f13726a2d3 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -3273,9 +3273,6 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]: ] SENTRY_MONOLITH_REGION = SENTRY_CELLS[0]["name"] - # TODO(cells): remove after getsentry updated - SENTRY_REGION_CONFIG = SENTRY_CELLS - # Cross region RPC authentication RPC_SHARED_SECRET = [ "a-long-value-that-is-shared-but-also-secret", diff --git a/src/sentry/integrations/api/endpoints/organization_integration_request.py b/src/sentry/integrations/api/endpoints/organization_integration_request.py index de30dc938e3c..c7fb4b2f2e0d 100644 --- a/src/sentry/integrations/api/endpoints/organization_integration_request.py +++ b/src/sentry/integrations/api/endpoints/organization_integration_request.py @@ -1,5 +1,7 @@ from __future__ import annotations +from typing import TYPE_CHECKING + from rest_framework.request import Request from rest_framework.response import Response @@ -15,8 +17,11 @@ from sentry.plugins.base import plugins from sentry.sentry_apps.services.app import app_service +if TYPE_CHECKING: + from django.utils.functional import _StrPromise + -def get_provider_name(provider_type: str, provider_slug: str) -> str | None: +def get_provider_name(provider_type: str, provider_slug: str) -> str | _StrPromise | None: """ The things that users think of as "integrations" are actually three different things: integrations, plugins, and sentryapps. A user requesting diff --git a/src/sentry/integrations/github/webhook_types.py b/src/sentry/integrations/github/webhook_types.py index 0ad1061471a0..242b201da7b3 100644 --- a/src/sentry/integrations/github/webhook_types.py +++ b/src/sentry/integrations/github/webhook_types.py @@ -22,7 +22,10 @@ class GithubWebhookType(StrEnum): # Event type strings (X-GitHub-Event header values) that the cell webhook endpoint processes. -# INSTALLATION is handled in control only. +# INSTALLATION and INSTALLATION_REPOSITORIES are handled in control only. +_CONTROL_ONLY_EVENTS = frozenset( + {GithubWebhookType.INSTALLATION, GithubWebhookType.INSTALLATION_REPOSITORIES} +) CELL_PROCESSED_GITHUB_EVENTS = frozenset( - t.value for t in GithubWebhookType if t != GithubWebhookType.INSTALLATION + t.value for t in GithubWebhookType if t not in _CONTROL_ONLY_EVENTS ) diff --git a/src/sentry/middleware/integrations/parsers/github.py b/src/sentry/middleware/integrations/parsers/github.py index c6ec3f3c5bb1..d8c1bbb5f106 100644 --- a/src/sentry/middleware/integrations/parsers/github.py +++ b/src/sentry/middleware/integrations/parsers/github.py @@ -15,9 +15,9 @@ get_github_external_id, ) from sentry.integrations.github.webhook_types import ( + _CONTROL_ONLY_EVENTS, CELL_PROCESSED_GITHUB_EVENTS, GITHUB_WEBHOOK_TYPE_HEADER, - GithubWebhookType, ) from sentry.integrations.middleware.hybrid_cloud.parser import BaseRequestParser from sentry.integrations.models.integration import Integration @@ -77,7 +77,7 @@ def get_mailbox_identifier( def should_route_to_control_silo( self, parsed_event: Mapping[str, Any], request: HttpRequest ) -> bool: - return request.META.get(GITHUB_WEBHOOK_TYPE_HEADER) == GithubWebhookType.INSTALLATION + return request.META.get(GITHUB_WEBHOOK_TYPE_HEADER) in _CONTROL_ONLY_EVENTS @control_silo_function def get_integration_from_request(self) -> Integration | None: diff --git a/src/sentry/notifications/notification_action/metric_alert_registry/handlers/slack_metric_alert_handler.py b/src/sentry/notifications/notification_action/metric_alert_registry/handlers/slack_metric_alert_handler.py index 04cd547b9d76..704f7cc8246d 100644 --- a/src/sentry/notifications/notification_action/metric_alert_registry/handlers/slack_metric_alert_handler.py +++ b/src/sentry/notifications/notification_action/metric_alert_registry/handlers/slack_metric_alert_handler.py @@ -1,12 +1,19 @@ import logging -from sentry.incidents.models.incident import TriggerStatus +import sentry_sdk + +from sentry import features +from sentry.incidents.charts import build_metric_alert_chart +from sentry.incidents.endpoints.serializers.alert_rule import AlertRuleSerializerResponse +from sentry.incidents.endpoints.serializers.incident import DetailedIncidentSerializerResponse +from sentry.incidents.models.incident import IncidentStatus, TriggerStatus from sentry.incidents.typings.metric_detector import ( AlertContext, MetricIssueContext, NotificationContext, OpenPeriodContext, ) +from sentry.integrations.metric_alerts import incident_attachment_info from sentry.integrations.slack.utils.notifications import send_incident_alert_notification from sentry.models.groupopenperiod import GroupOpenPeriod from sentry.models.organization import Organization @@ -18,11 +25,106 @@ ) from sentry.notifications.notification_action.registry import metric_alert_handler_registry from sentry.notifications.notification_action.types import BaseMetricAlertHandler +from sentry.notifications.platform.service import NotificationService +from sentry.notifications.platform.target import IntegrationNotificationTarget +from sentry.notifications.platform.templates.metric_alert import MetricAlertNotificationData +from sentry.notifications.platform.threading import ThreadingOptions, ThreadKey +from sentry.notifications.platform.types import ( + NotificationProviderKey, + NotificationSource, + NotificationTargetResourceType, +) +from sentry.workflow_engine.endpoints.serializers.detector_serializer import ( + DetectorSerializerResponse, +) from sentry.workflow_engine.models import Action, Detector logger = logging.getLogger(__name__) +def _send_via_notification_platform( + notification_context: NotificationContext, + alert_context: AlertContext, + metric_issue_context: MetricIssueContext, + open_period_context: OpenPeriodContext, + notification_uuid: str, + organization: Organization, + alert_rule_serialized_response: AlertRuleSerializerResponse, + detector_serialized_response: DetectorSerializerResponse, + incident_serialized_response: DetailedIncidentSerializerResponse, +) -> None: + if notification_context.integration_id is None: + raise ValueError("Integration ID is None") + + if notification_context.target_identifier is None: + raise ValueError("Slack channel is None") + + attachment_info = incident_attachment_info( + organization=organization, + alert_context=alert_context, + metric_issue_context=metric_issue_context, + notification_uuid=notification_uuid, + referrer="metric_alert_slack", + ) + + chart_url = None + if ( + features.has("organizations:metric-alert-chartcuterie", organization) + and alert_rule_serialized_response + and incident_serialized_response + ): + try: + chart_url = build_metric_alert_chart( + organization=organization, + alert_rule_serialized_response=alert_rule_serialized_response, + snuba_query=metric_issue_context.snuba_query, + alert_context=alert_context, + open_period_context=open_period_context, + selected_incident_serialized=incident_serialized_response, + subscription=metric_issue_context.subscription, + detector_serialized_response=detector_serialized_response, + ) + except Exception as e: + sentry_sdk.capture_exception(e) + + data = MetricAlertNotificationData( + group_id=metric_issue_context.id, + organization_id=organization.id, + notification_uuid=notification_uuid, + action_id=notification_context.id, + open_period_context=open_period_context, + new_status=metric_issue_context.new_status.value, + title=attachment_info["title"], + title_link=attachment_info["title_link"], + text=attachment_info["text"], + chart_url=chart_url, + ) + + target = IntegrationNotificationTarget( + provider_key=NotificationProviderKey.SLACK, + resource_type=NotificationTargetResourceType.CHANNEL, + resource_id=notification_context.target_identifier, + integration_id=notification_context.integration_id, + organization_id=organization.id, + ) + + threading_options = ThreadingOptions( + thread_key=ThreadKey( + key_type=NotificationSource.METRIC_ALERT, + key_data={ + "action_id": notification_context.id, + "group_id": metric_issue_context.id, + "open_period_start": open_period_context.date_started.isoformat(), + }, + ), + reply_broadcast=(metric_issue_context.new_status == IncidentStatus.CRITICAL), + ) + + NotificationService[MetricAlertNotificationData](data=data).notify_sync( + targets=[target], threading_options=threading_options + ) + + @metric_alert_handler_registry.register(Action.Type.SLACK) class SlackMetricAlertHandler(BaseMetricAlertHandler): @classmethod @@ -58,17 +160,30 @@ def send_alert( }, ) - send_incident_alert_notification( - notification_context=notification_context, - alert_context=alert_context, - metric_issue_context=metric_issue_context, - open_period_context=open_period_context, - organization=organization, - notification_uuid=notification_uuid, - alert_rule_serialized_response=alert_rule_serialized_response, - incident_serialized_response=incident_serialized_response, - detector_serialized_response=detector_serialized_response, - ) + if NotificationService.has_access(organization, NotificationSource.METRIC_ALERT): + _send_via_notification_platform( + notification_context=notification_context, + alert_context=alert_context, + metric_issue_context=metric_issue_context, + open_period_context=open_period_context, + notification_uuid=notification_uuid, + organization=organization, + alert_rule_serialized_response=alert_rule_serialized_response, + detector_serialized_response=detector_serialized_response, + incident_serialized_response=incident_serialized_response, + ) + else: + send_incident_alert_notification( + notification_context=notification_context, + alert_context=alert_context, + metric_issue_context=metric_issue_context, + open_period_context=open_period_context, + organization=organization, + notification_uuid=notification_uuid, + alert_rule_serialized_response=alert_rule_serialized_response, + incident_serialized_response=incident_serialized_response, + detector_serialized_response=detector_serialized_response, + ) @metric_alert_handler_registry.register(Action.Type.SLACK_STAGING) diff --git a/src/sentry/notifications/notification_action/utils.py b/src/sentry/notifications/notification_action/utils.py index 6b3e4396e13f..1962848e444f 100644 --- a/src/sentry/notifications/notification_action/utils.py +++ b/src/sentry/notifications/notification_action/utils.py @@ -124,13 +124,16 @@ def issue_notification_data_factory(invocation: ActionInvocation) -> IssueNotifi detector=detector, event_data=event_data, ) - rule_instance.data["tags"] = action.data.get("tags", "") - rule_instance.data["notes"] = action.data.get("notes", "") + tags = action.data.get("tags", None) + tag_list = [tag.strip() for tag in tags.split(",")] if tags else None + notes = action.data.get("notes", None) rule = SerializableRuleProxy.from_rule(rule_instance) event_id = getattr(event_data.event, "event_id", None) if event_data.event else None return IssueNotificationData( + tags=tag_list, + notes=notes, event_id=event_id, group_id=event_data.group.id, notification_uuid=invocation.notification_uuid, diff --git a/src/sentry/notifications/platform/discord/provider.py b/src/sentry/notifications/platform/discord/provider.py index cdbdeb372b76..b56ea624cfcb 100644 --- a/src/sentry/notifications/platform/discord/provider.py +++ b/src/sentry/notifications/platform/discord/provider.py @@ -21,6 +21,7 @@ NotificationBodyFormattingBlockType, NotificationBodyTextBlock, NotificationBodyTextBlockType, + NotificationCategory, NotificationData, NotificationProviderKey, NotificationRenderedTemplate, @@ -136,6 +137,16 @@ def is_available(cls, *, organization: RpcOrganizationSummary | None = None) -> # TODO(ecosystem): Check for the integration, maybe a feature as well return False + @classmethod + def get_renderer( + cls, *, data: NotificationData, category: NotificationCategory + ) -> type[NotificationRenderer[DiscordRenderable]]: + from sentry.notifications.platform.discord.renderers.issue import IssueDiscordRenderer + + if category == NotificationCategory.ISSUE: + return IssueDiscordRenderer + return cls.default_renderer + @classmethod def send( cls, diff --git a/src/sentry/notifications/platform/discord/renderers/__init__.py b/src/sentry/notifications/platform/discord/renderers/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/src/sentry/notifications/platform/discord/renderers/issue.py b/src/sentry/notifications/platform/discord/renderers/issue.py new file mode 100644 index 000000000000..855f2a163cec --- /dev/null +++ b/src/sentry/notifications/platform/discord/renderers/issue.py @@ -0,0 +1,59 @@ +from __future__ import annotations + +from sentry import eventstore +from sentry.integrations.discord.message_builder.issues import DiscordIssuesMessageBuilder +from sentry.models.group import Group +from sentry.notifications.platform.discord.provider import DiscordRenderable +from sentry.notifications.platform.renderer import NotificationRenderer +from sentry.notifications.platform.service import NotificationRenderError +from sentry.notifications.platform.templates.issue import IssueNotificationData +from sentry.notifications.platform.types import ( + NotificationData, + NotificationProviderKey, + NotificationRenderedTemplate, +) +from sentry.services.eventstore.models import Event + + +class IssueDiscordRenderer(NotificationRenderer[DiscordRenderable]): + provider_key = NotificationProviderKey.DISCORD + + @classmethod + def render[DataT: NotificationData]( + cls, *, data: DataT, rendered_template: NotificationRenderedTemplate + ) -> DiscordRenderable: + if not isinstance(data, IssueNotificationData): + raise ValueError(f"IssueDiscordRenderer does not support {data.__class__.__name__}") + + # Retrieving Group and Event data is an anti-pattern, do not do this + # in permanent renderers. + try: + group = Group.objects.get_from_cache(id=data.group_id) + except Group.DoesNotExist: + raise NotificationRenderError(f"Group {data.group_id} not found") + + group_event = None + if data.event_id: + try: + event = eventstore.backend.get_event_by_id( + project_id=group.project.id, event_id=data.event_id, group_id=data.group_id + ) + if isinstance(event, Event): + # Discord only supports GroupEvents, and we can't guarantee + # the type passed by eventstore, so we convert base Events + # to GroupEvents. + group_event = event.for_group(group) + else: + group_event = event + except Exception: + raise NotificationRenderError(f"Failed to retrieve event {data.event_id}") + + rules = [data.rule.to_rule()] if data.rule else [] + + return DiscordIssuesMessageBuilder( + group=group, + event=group_event, + tags=set(data.tags) if data.tags else None, + rules=rules, + link_to_event=True, + ).build(notification_uuid=data.notification_uuid) diff --git a/src/sentry/notifications/platform/slack/renderers/issue.py b/src/sentry/notifications/platform/slack/renderers/issue.py index 46182c5cc77d..096fd3b73513 100644 --- a/src/sentry/notifications/platform/slack/renderers/issue.py +++ b/src/sentry/notifications/platform/slack/renderers/issue.py @@ -29,13 +29,12 @@ def render[DataT: NotificationData]( if data.event_id: event = eventstore.backend.get_event_by_id(group.project.id, data.event_id) - tags = data.rule.data.get("tags", None) or None blocks_dict = SlackIssuesMessageBuilder( group=group, event=event, - tags=set(tag.strip() for tag in tags.split(",")) if tags else None, + tags=set(data.tags) if data.tags else None, rules=[data.rule.to_rule()] if data.rule else None, - notes=data.rule.data.get("notes", None) or None, + notes=data.notes, link_to_event=True, ).build(notification_uuid=data.notification_uuid) diff --git a/src/sentry/notifications/platform/templates/issue.py b/src/sentry/notifications/platform/templates/issue.py index 128943657da0..1873bd6c9a3e 100644 --- a/src/sentry/notifications/platform/templates/issue.py +++ b/src/sentry/notifications/platform/templates/issue.py @@ -61,6 +61,8 @@ class IssueNotificationData(NotificationData): group_id: int event_id: str | None = None + tags: list[str] | None = None + notes: str | None = None rule: SerializableRuleProxy notification_uuid: str = "" @@ -72,14 +74,14 @@ class IssueNotificationTemplate(NotificationTemplate[IssueNotificationData]): group_id=1, event_id="abc123", notification_uuid="test-uuid", + tags=["environment", "level"], + notes="example note", rule=SerializableRuleProxy( id=1, project_id=2, label="Example Rule", data={ "actions": [{"workflow_id": 3}], - "tags": "environment,level", - "notes": "example note", }, ), ) diff --git a/src/sentry/plugins/base/manager.py b/src/sentry/plugins/base/manager.py index 448432b4ce9a..2cab7ce74579 100644 --- a/src/sentry/plugins/base/manager.py +++ b/src/sentry/plugins/base/manager.py @@ -61,7 +61,7 @@ def for_project(self, project, version=1) -> Generator[Plugin | Plugin2]: continue yield plugin - def get(self, slug): + def get(self, slug: str) -> Plugin | Plugin2: for plugin in self.all(version=None): if plugin.slug == slug: return plugin diff --git a/src/sentry/plugins/interfaces/releasehook.py b/src/sentry/plugins/interfaces/releasehook.py index a8cd3abd0d42..65de6fec8795 100644 --- a/src/sentry/plugins/interfaces/releasehook.py +++ b/src/sentry/plugins/interfaces/releasehook.py @@ -3,9 +3,9 @@ import logging from django.db import IntegrityError, router, transaction +from django.http import HttpRequest from django.http.response import HttpResponseBase from django.utils import timezone -from rest_framework.request import Request from sentry.exceptions import HookValidationError from sentry.models.activity import Activity @@ -79,5 +79,5 @@ def finish_release(self, version, **values): ) self.set_refs(release=release, **values) - def handle(self, request: Request) -> HttpResponseBase | None: + def handle(self, request: HttpRequest) -> HttpResponseBase | None: raise NotImplementedError diff --git a/src/sentry/preprod/vcs/status_checks/snapshots/tasks.py b/src/sentry/preprod/vcs/status_checks/snapshots/tasks.py index fce89a853807..30accd444b3a 100644 --- a/src/sentry/preprod/vcs/status_checks/snapshots/tasks.py +++ b/src/sentry/preprod/vcs/status_checks/snapshots/tasks.py @@ -239,6 +239,7 @@ def create_preprod_snapshot_status_check_task( status, base_artifact_map, changes_map, + approvals_map=approvals_map, ) if any(changes_map.values()): approve_action_identifier = APPROVE_SNAPSHOT_ACTION_IDENTIFIER diff --git a/src/sentry/preprod/vcs/status_checks/snapshots/templates.py b/src/sentry/preprod/vcs/status_checks/snapshots/templates.py index 2555a7209935..37a655c4d7cc 100644 --- a/src/sentry/preprod/vcs/status_checks/snapshots/templates.py +++ b/src/sentry/preprod/vcs/status_checks/snapshots/templates.py @@ -4,7 +4,7 @@ from django.utils.translation import ngettext from sentry.integrations.source_code_management.status_check import StatusCheckStatus -from sentry.preprod.models import PreprodArtifact +from sentry.preprod.models import PreprodArtifact, PreprodComparisonApproval from sentry.preprod.snapshots.models import PreprodSnapshotComparison, PreprodSnapshotMetrics from sentry.preprod.url_utils import get_preprod_artifact_comparison_url, get_preprod_artifact_url @@ -19,6 +19,7 @@ def format_snapshot_status_check_messages( overall_status: StatusCheckStatus, base_artifact_map: dict[int, PreprodArtifact], changes_map: dict[int, bool], + approvals_map: dict[int, PreprodComparisonApproval] | None = None, ) -> tuple[str, str, str]: if not artifacts: raise ValueError("Cannot format messages for empty artifact list") @@ -110,6 +111,7 @@ def format_snapshot_status_check_messages( comparisons_map, base_artifact_map, changes_map, + approvals_map=approvals_map, ) return str(title), str(subtitle), str(summary) @@ -222,6 +224,7 @@ def _format_snapshot_summary( comparisons_map: dict[int, PreprodSnapshotComparison], base_artifact_map: dict[int, PreprodArtifact], changes_map: dict[int, bool], + approvals_map: dict[int, PreprodComparisonApproval] | None = None, ) -> str: table_rows = [] @@ -268,7 +271,13 @@ def _format_snapshot_summary( renamed = comparison.images_renamed unchanged = comparison.images_unchanged has_changes = changes_map.get(artifact.id, False) - status = "⏳ Needs approval" if has_changes else "✅ Unchanged" + is_approved = approvals_map is not None and artifact.id in approvals_map + if has_changes and is_approved: + status = "✅ Approved" + elif has_changes: + status = "⏳ Needs approval" + else: + status = "✅ Unchanged" def _section_cell(count: int, section: str) -> str: if count > 0: diff --git a/src/sentry/seer/autofix/utils.py b/src/sentry/seer/autofix/utils.py index f0bf410ef221..1e72a58fc7e9 100644 --- a/src/sentry/seer/autofix/utils.py +++ b/src/sentry/seer/autofix/utils.py @@ -561,9 +561,12 @@ def _write_preference_project_options(project: Project, preference: SeerProjectP project.update_option( "sentry:seer_automation_handoff_integration_id", handoff.integration_id ) - project.update_option( - "sentry:seer_automation_handoff_auto_create_pr", handoff.auto_create_pr - ) + if handoff.auto_create_pr: + project.update_option( + "sentry:seer_automation_handoff_auto_create_pr", handoff.auto_create_pr + ) + else: + project.delete_option("sentry:seer_automation_handoff_auto_create_pr") else: project.delete_option("sentry:seer_automation_handoff_point") project.delete_option("sentry:seer_automation_handoff_target") diff --git a/src/sentry/seer/endpoints/organization_seer_explorer_chat.py b/src/sentry/seer/endpoints/organization_seer_explorer_chat.py index f4925560a747..a0ba5be568ce 100644 --- a/src/sentry/seer/endpoints/organization_seer_explorer_chat.py +++ b/src/sentry/seer/endpoints/organization_seer_explorer_chat.py @@ -15,10 +15,14 @@ from sentry.models.organization import Organization from sentry.ratelimits.config import RateLimitConfig from sentry.seer.explorer.client import SeerExplorerClient -from sentry.seer.explorer.client_utils import has_seer_explorer_access_with_detail +from sentry.seer.explorer.client_utils import ( + has_seer_explorer_access_with_detail, + snapshot_to_markdown, +) from sentry.seer.models import SeerPermissionError from sentry.seer.seer_setup import has_seer_access_with_detail from sentry.types.ratelimit import RateLimit, RateLimitCategory +from sentry.utils import json logger = logging.getLogger(__name__) @@ -153,6 +157,15 @@ def post( page_name = validated_data.get("page_name") override_ce_enable = validated_data["override_ce_enable"] + # If the frontend sent a structured LLMContext JSON snapshot, convert to markdown. + if on_page_context: + try: + snapshot = json.loads(on_page_context) + if isinstance(snapshot, dict) and "nodes" in snapshot: + on_page_context = snapshot_to_markdown(snapshot) + except (json.JSONDecodeError, TypeError, AttributeError): + pass + try: enable_coding = organization.get_option( "sentry:enable_seer_coding", False diff --git a/src/sentry/seer/explorer/client_utils.py b/src/sentry/seer/explorer/client_utils.py index 191a45771203..c05971dab8bc 100644 --- a/src/sentry/seer/explorer/client_utils.py +++ b/src/sentry/seer/explorer/client_utils.py @@ -340,3 +340,36 @@ def poll_until_done( # Wait before next poll time.sleep(poll_interval) + + +def _render_node(node: dict[str, Any], depth: int) -> str: + """Recursively render an LLMContextSnapshot node and its children as markdown.""" + heading = "#" * min(depth + 1, 6) + lines = [f"{heading} {node.get('nodeType', 'unknown')}"] + + data = node.get("data") + if isinstance(data, dict): + for key, value in data.items(): + lines.append(f"- **{key}**: {orjson.dumps(value).decode()}") + elif data is not None: + lines.append(f"- {orjson.dumps(data).decode()}") + + for child in node.get("children", []): + lines.append(_render_node(child, depth + 1)) + + return "\n".join(lines) + + +def snapshot_to_markdown(snapshot: dict[str, Any]) -> str: + """Convert an LLMContextSnapshot dict to a markdown string. + + Expected shape: ``{"version": int, "nodes": [{"nodeType": str, "data": ..., "children": [...]}]}`` + The top-level nodes list contains a single root node (the page). + """ + nodes = snapshot.get("nodes", []) + if not nodes: + return "" + preamble = ( + "> This is a structured summary of the page the user is viewing, not an exact screenshot.\n" + ) + return preamble + _render_node(nodes[0], 0) diff --git a/src/sentry/seer/signed_seer_api.py b/src/sentry/seer/signed_seer_api.py index 20cf09976c87..1563cc7f9e49 100644 --- a/src/sentry/seer/signed_seer_api.py +++ b/src/sentry/seer/signed_seer_api.py @@ -11,6 +11,7 @@ from sentry.net.http import connection_from_url from sentry.utils import metrics +from sentry.viewer_context import ViewerContext, get_viewer_context class SeerViewerContext(TypedDict, total=False): @@ -47,6 +48,68 @@ class SeerViewerContext(TypedDict, total=False): ) +def _resolve_viewer_context( + explicit: SeerViewerContext | None = None, +) -> ViewerContext | None: + """Merge explicit SeerViewerContext with the contextvar. + + Converts the legacy SeerViewerContext into a ViewerContext, then merges + with the contextvar. Explicit non-None fields win. On disagreement, + logs a warning and strips the token for safety. + """ + vc = get_viewer_context() + + if explicit is None and vc is None: + return None + if explicit is None: + return vc + + explicit_vc = ViewerContext( + organization_id=explicit.get("organization_id"), + user_id=explicit.get("user_id"), + ) + + if vc is None: + return explicit_vc + + has_mismatch = False + org_id = vc.organization_id + user_id = vc.user_id + + if explicit_vc.organization_id is not None: + if org_id is not None and org_id != explicit_vc.organization_id: + logger.warning( + "seer.viewer_context_mismatch", + extra={ + "field": "organization_id", + "contextvar": org_id, + "explicit": explicit_vc.organization_id, + }, + ) + has_mismatch = True + org_id = explicit_vc.organization_id + + if explicit_vc.user_id is not None: + if user_id is not None and user_id != explicit_vc.user_id: + logger.warning( + "seer.viewer_context_mismatch", + extra={ + "field": "user_id", + "contextvar": user_id, + "explicit": explicit_vc.user_id, + }, + ) + has_mismatch = True + user_id = explicit_vc.user_id + + return ViewerContext( + organization_id=org_id, + user_id=user_id, + actor_type=vc.actor_type, + token=None if has_mismatch else vc.token, + ) + + @sentry_sdk.tracing.trace def make_signed_seer_api_request( connection_pool: HTTPConnectionPool, @@ -72,10 +135,11 @@ def make_signed_seer_api_request( **auth_headers, } - if viewer_context: + resolved = _resolve_viewer_context(viewer_context) + if resolved: if settings.SEER_API_SHARED_SECRET: try: - context_bytes = orjson.dumps(viewer_context) + context_bytes = orjson.dumps(resolved.serialize()) context_signature = sign_viewer_context(context_bytes) headers["X-Viewer-Context"] = context_bytes.decode("utf-8") headers["X-Viewer-Context-Signature"] = context_signature diff --git a/src/sentry/tagstore/snuba/backend.py b/src/sentry/tagstore/snuba/backend.py index 1df6578b8626..2048d20eb963 100644 --- a/src/sentry/tagstore/snuba/backend.py +++ b/src/sentry/tagstore/snuba/backend.py @@ -40,8 +40,10 @@ 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.columns import datetime_processor from sentry.search.eap.occurrences.common_queries import count_occurrences from sentry.search.eap.occurrences.definitions import OCCURRENCE_DEFINITIONS +from sentry.search.eap.occurrences.query_utils import build_escaped_term_filter from sentry.search.eap.occurrences.rollout_utils import EAPOccurrencesComparator from sentry.search.eap.resolver import SearchResolver from sentry.search.eap.types import SearchResolverConfig @@ -145,6 +147,86 @@ def _reasonable_user_counts_match(control: dict[int, int], experimental: dict[in return all(experimental[group_id] <= control[group_id] for group_id in experimental) +def _reasonable_release_tags_match(control: set[TagValue], experimental: set[TagValue]) -> bool: + exp_by_value: dict[str | None, TagValue] = {tv.value: tv for tv in experimental} + ctrl_by_value: dict[str | None, TagValue] = {} + for tv in control: + existing = ctrl_by_value.get(tv.value) + if existing is None or (tv.times_seen or 0) > (existing.times_seen or 0): + ctrl_by_value[tv.value] = tv + + if not set(exp_by_value.keys()).issubset(set(ctrl_by_value.keys())): + return False + + return all( + (exp_tv.times_seen or 0) <= (ctrl_by_value[value].times_seen or 0) + for value, exp_tv in exp_by_value.items() + ) + + +def _reasonable_group_list_tag_value_match( + control: dict[int, GroupTagValue], + experimental: dict[int, GroupTagValue], +) -> bool: + if not set(experimental.keys()).issubset(set(control.keys())): + return False + for group_id in experimental: + ctrl = control[group_id] + exp = experimental[group_id] + if exp.times_seen > ctrl.times_seen: + return False + if ( + exp.first_seen is not None + and ctrl.first_seen is not None + and exp.first_seen < ctrl.first_seen + ): + return False + if ( + exp.last_seen is not None + and ctrl.last_seen is not None + and exp.last_seen > ctrl.last_seen + ): + return False + return True + + +_SNUBA_TO_EAP_ORDERBY = { + "first_seen": "min(timestamp)", + "-first_seen": "-min(timestamp)", + "last_seen": "last_seen()", + "-last_seen": "-last_seen()", + "times_seen": "count()", + "-times_seen": "-count()", +} + + +def _reasonable_group_tag_value_iter_match( + control: Sequence[GroupTagValue], + experimental: Sequence[GroupTagValue], +) -> bool: + ctrl_by_value = {tv.value: tv for tv in control} + exp_by_value = {tv.value: tv for tv in experimental} + if not set(exp_by_value.keys()).issubset(set(ctrl_by_value.keys())): + return False + for value, exp_tv in exp_by_value.items(): + ctrl_tv = ctrl_by_value[value] + if exp_tv.times_seen > ctrl_tv.times_seen: + return False + if ( + exp_tv.first_seen is not None + and ctrl_tv.first_seen is not None + and exp_tv.first_seen < ctrl_tv.first_seen + ): + return False + if ( + exp_tv.last_seen is not None + and ctrl_tv.last_seen is not None + and exp_tv.last_seen > ctrl_tv.last_seen + ): + return False + return True + + class _OptimizeKwargs(TypedDict, total=False): turbo: bool sample: int @@ -806,7 +888,7 @@ def get_group_list_tag_value( value, tenant_ids=None, ): - return self.__get_group_list_tag_value( + snuba_result = self.__get_group_list_tag_value( project_ids, group_id_list, environment_ids, @@ -815,9 +897,38 @@ def get_group_list_tag_value( Dataset.Events, [DEFAULT_TYPE_CONDITION], [], - "tagstore.get_group_list_tag_value", + Referrer.TAGSTORE_GET_GROUP_LIST_TAG_VALUE.value, tenant_ids=tenant_ids, ) + result = snuba_result + + callsite = "SnubaTagStorage::get_group_list_tag_value" + if EAPOccurrencesComparator.should_check_experiment(callsite): + eap_result = self._eap_get_group_list_tag_value( + project_ids, + group_id_list, + environment_ids, + key, + value, + referrer=Referrer.TAGSTORE_GET_GROUP_LIST_TAG_VALUE.value, + 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_group_list_tag_value_match, + debug_context={ + "project_ids": list(project_ids), + "group_ids": list(group_id_list), + "environment_ids": list(environment_ids) if environment_ids else None, + "key": key, + "value": value, + }, + ) + + return result def get_generic_group_list_tag_value( self, @@ -871,12 +982,137 @@ def get_generic_group_list_tag_value( result_snql["data"], ["group_id"], ["times_seen", "first_seen", "last_seen"] ) - return { + snuba_result = { group_id: GroupTagValue( group_id=group_id, key=key, value=value, **fix_tag_value_data(data) ) for group_id, data in nested_groups.items() } + result = snuba_result + + callsite = "SnubaTagStorage::get_generic_group_list_tag_value" + if EAPOccurrencesComparator.should_check_experiment(callsite): + eap_result = self._eap_get_group_list_tag_value( + project_ids, + group_id_list, + environment_ids, + key, + value, + referrer=Referrer.TAGSTORE_GET_GENERIC_GROUP_LIST_TAG_VALUE.value, + 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_group_list_tag_value_match, + debug_context={ + "project_ids": list(project_ids), + "group_ids": list(group_id_list), + "environment_ids": list(environment_ids) if environment_ids else None, + "key": key, + "value": value, + }, + ) + + return result + + def _eap_get_group_list_tag_value( + self, + project_ids: Sequence[int], + group_id_list: Sequence[int], + environment_ids: Sequence[int] | None, + key: str, + value: str, + referrer: str, + occurrence_category: OccurrenceCategory, + ) -> dict[int, GroupTagValue]: + organization_id = get_organization_id_from_project_ids(project_ids) + + now = datetime.now(tz=timezone.utc) + resolved_start = now - timedelta(days=90) + resolved_end = now + + try: + organization = Organization.objects.get_from_cache(id=organization_id) + except Organization.DoesNotExist: + return {} + + projects = list(Project.objects.filter(id__in=project_ids, organization_id=organization_id)) + if not projects: + return {} + + environments = ( + list(Environment.objects.filter(id__in=environment_ids)) if environment_ids else [] + ) + + group_id_filter = f"group_id:[{','.join(str(gid) for gid in group_id_list)}]" + tag_filter = build_escaped_term_filter(key, [str(value)]) + query_string = f"{group_id_filter} {tag_filter}" + + 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()", "min(timestamp)", "last_seen()"], + orderby=None, + offset=0, + limit=len(group_id_list), + referrer=referrer, + config=SearchResolverConfig(), + occurrence_category=occurrence_category, + ) + + output: dict[int, GroupTagValue] = {} + for row in result.get("data", []): + group_id = row.get("group_id") + if group_id is None: + continue + first_seen_raw = row.get("min(timestamp)") + last_seen_raw = row.get("last_seen()") + output[int(group_id)] = GroupTagValue( + group_id=int(group_id), + key=key, + value=value, + times_seen=int(row.get("count()", 0)), + first_seen=( + parse_datetime(datetime_processor(first_seen_raw)).replace( + tzinfo=timezone.utc + ) + if first_seen_raw is not None + else None + ), + last_seen=( + parse_datetime(datetime_processor(last_seen_raw)).replace( + tzinfo=timezone.utc + ) + if last_seen_raw is not None + else None + ), + ) + return output + except Exception: + logger.exception( + "EAP get_group_list_tag_value query failed", + extra={ + "organization_id": organization_id, + "project_ids": list(project_ids), + "group_ids": list(group_id_list), + "key": key, + "value": value, + "occurrence_category": occurrence_category.value, + }, + ) + return {} def apply_group_filters(self, group: Group | None, filters: MutableMapping[str, Sequence[Any]]): dataset = Dataset.Events @@ -1136,7 +1372,7 @@ def get_release_tags(self, organization_id, project_ids, environment_id, version ["max", SEEN_COLUMN, "last_seen"], ] start = self.get_min_start_date(organization_id, project_ids, environment_id, versions) - result = snuba.query( + snuba_result_raw = snuba.query( dataset=Dataset.Events, start=start, groupby=["project_id", col], @@ -1144,16 +1380,130 @@ def get_release_tags(self, organization_id, project_ids, environment_id, version filter_keys=filters, aggregations=aggregations, orderby="-times_seen", - referrer="tagstore.get_release_tags", + referrer=Referrer.TAGSTORE_GET_RELEASE_TAGS.value, tenant_ids={"organization_id": organization_id}, ) values = [] - for project_data in result.values(): + for project_data in snuba_result_raw.values(): for value, data in project_data.items(): values.append(TagValue(key=tag, value=value, **fix_tag_value_data(data))) - return set(values) + snuba_result = set(values) + result = snuba_result + + callsite = "SnubaTagStorage::get_release_tags" + if EAPOccurrencesComparator.should_check_experiment(callsite): + eap_result = self._eap_get_release_tags( + organization_id, project_ids, environment_id, versions, start + ) + 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_release_tags_match, + debug_context={ + "organization_id": organization_id, + "project_ids": list(project_ids), + "environment_id": environment_id, + "versions": list(versions), + }, + ) + + return result + + def _eap_get_release_tags( + self, + organization_id: int, + project_ids: Sequence[int], + environment_id: int | None, + versions: Sequence[str], + start: datetime | None, + ) -> set[TagValue]: + try: + organization = Organization.objects.get_from_cache(id=organization_id) + except Organization.DoesNotExist: + return set() + + projects = list(Project.objects.filter(id__in=project_ids, organization_id=organization_id)) + if not projects: + return set() + + environments = list(Environment.objects.filter(id=environment_id)) if environment_id else [] + + now = datetime.now(tz=timezone.utc) + resolved_start = start if start is not None else now - timedelta(days=90) + + query_string = build_escaped_term_filter("release", [str(v) for v in versions]) + + snuba_params = SnubaParams( + start=resolved_start, + end=now, + organization=organization, + projects=projects, + environments=environments, + ) + + try: + result = Occurrences.run_table_query( + params=snuba_params, + query_string=query_string, + selected_columns=[ + "project_id", + "release", + "count()", + "min(timestamp)", + "last_seen()", + ], + orderby=["-count()"], + offset=0, + limit=len(versions) * len(project_ids), + referrer=Referrer.TAGSTORE_GET_RELEASE_TAGS.value, + config=SearchResolverConfig(), + occurrence_category=OccurrenceCategory.ERROR, + ) + + tag = "sentry:release" + tag_values = [] + for row in result.get("data", []): + release_val = row.get("release") + if release_val is None: + continue + first_seen_raw = row.get("min(timestamp)") + last_seen_raw = row.get("last_seen()") + tag_values.append( + TagValue( + key=tag, + value=release_val, + times_seen=int(row.get("count()", 0)), + first_seen=( + parse_datetime(datetime_processor(first_seen_raw)).replace( + tzinfo=timezone.utc + ) + if first_seen_raw is not None + else None + ), + last_seen=( + parse_datetime(datetime_processor(last_seen_raw)).replace( + tzinfo=timezone.utc + ) + if last_seen_raw is not None + else None + ), + ) + ) + return set(tag_values) + except Exception: + logger.exception( + "EAP get_release_tags query failed", + extra={ + "organization_id": organization_id, + "project_ids": list(project_ids), + "versions": list(versions), + }, + ) + return set() def get_min_start_date( self, organization_id, project_ids, environment_id, versions @@ -1905,17 +2255,151 @@ def get_group_tag_value_iter( ], orderby=orderby, limit=limit, - referrer="tagstore.get_group_tag_value_iter", + referrer=Referrer.TAGSTORE_GET_GROUP_TAG_VALUE_ITER.value, offset=offset, tenant_ids=tenant_ids, ) - group_tag_values = [ + snuba_result = [ GroupTagValue(group_id=group.id, key=key, value=value, **fix_tag_value_data(data)) for value, data in results.items() ] + result = snuba_result - return group_tag_values + callsite = "SnubaTagStorage::get_group_tag_value_iter" + if EAPOccurrencesComparator.should_check_experiment(callsite): + occurrence_category = ( + OccurrenceCategory.ERROR + if group.issue_category == GroupCategory.ERROR + else OccurrenceCategory.ISSUE_PLATFORM + ) + eap_result = self._eap_get_group_tag_value_iter( + group, + environment_ids, + key, + orderby, + limit, + offset, + 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=len(eap_result) == 0, + reasonable_match_comparator=_reasonable_group_tag_value_iter_match, + debug_context={ + "group_id": group.id, + "project_id": group.project_id, + "environment_ids": list(environment_ids) if environment_ids else None, + "key": key, + "orderby": orderby, + "limit": limit, + "offset": offset, + }, + ) + + return result + + def _eap_get_group_tag_value_iter( + self, + group: Group, + environment_ids: Sequence[int | None], + key: str, + orderby: str, + limit: int, + offset: int, + occurrence_category: OccurrenceCategory, + ) -> list[GroupTagValue]: + organization_id = group.project.organization_id + + now = datetime.now(tz=timezone.utc) + resolved_start = now - timedelta(days=90) + resolved_end = now + + try: + organization = Organization.objects.get_from_cache(id=organization_id) + except Organization.DoesNotExist: + return [] + + filtered_env_ids = ( + [eid for eid in environment_ids if eid is not None] if environment_ids else [] + ) + environments = ( + list(Environment.objects.filter(id__in=filtered_env_ids)) if filtered_env_ids else [] + ) + + query_string = f"group_id:{group.id}" + + eap_orderby_value = _SNUBA_TO_EAP_ORDERBY.get(orderby) + eap_orderby = [eap_orderby_value] if eap_orderby_value else None + + snuba_params = SnubaParams( + start=resolved_start, + end=resolved_end, + organization=organization, + projects=[group.project], + environments=environments, + ) + + eap_tag_column = format_attr_key(key) + + try: + result = Occurrences.run_table_query_with_tags( + {eap_tag_column}, + params=snuba_params, + query_string=query_string, + selected_columns=[eap_tag_column, "count()", "min(timestamp)", "last_seen()"], + orderby=eap_orderby, + offset=offset, + limit=limit, + referrer=Referrer.TAGSTORE_GET_GROUP_TAG_VALUE_ITER.value, + config=SearchResolverConfig(), + occurrence_category=occurrence_category, + ) + + output: list[GroupTagValue] = [] + for row in result.get("data", []): + tag_value = row.get(eap_tag_column) + if tag_value is None: + continue + first_seen_raw = row.get("min(timestamp)") + last_seen_raw = row.get("last_seen()") + output.append( + GroupTagValue( + group_id=group.id, + key=key, + value=tag_value, + times_seen=int(row.get("count()", 0)), + first_seen=( + parse_datetime(datetime_processor(first_seen_raw)).replace( + tzinfo=timezone.utc + ) + if first_seen_raw is not None + else None + ), + last_seen=( + parse_datetime(datetime_processor(last_seen_raw)).replace( + tzinfo=timezone.utc + ) + if last_seen_raw is not None + else None + ), + ) + ) + return output + except Exception: + logger.exception( + "EAP get_group_tag_value_iter query failed", + extra={ + "organization_id": organization_id, + "group_id": group.id, + "project_id": group.project_id, + "key": key, + "occurrence_category": occurrence_category.value, + }, + ) + return [] def get_group_tag_value_paginator( self, diff --git a/src/sentry/tasks/post_process.py b/src/sentry/tasks/post_process.py index d9076e4616d6..f4d8390e65c6 100644 --- a/src/sentry/tasks/post_process.py +++ b/src/sentry/tasks/post_process.py @@ -6,7 +6,7 @@ from collections.abc import MutableMapping, Sequence from datetime import datetime from time import time -from typing import TYPE_CHECKING, Any, TypedDict +from typing import TYPE_CHECKING, Any, Callable, TypedDict import sentry_sdk from django.conf import settings @@ -72,7 +72,7 @@ class PostProcessJob(TypedDict, total=False): has_escalated: bool -def _should_send_error_created_hooks(project): +def _should_send_error_created_hooks(project: Project) -> bool: from sentry.models.organization import Organization from sentry.sentry_apps.models.servicehook import ServiceHook @@ -94,33 +94,34 @@ def _should_send_error_created_hooks(project): cache_value = 1 if result else 0 cache.set(cache_key, cache_value, 60) - return result + # We cache either 0 or 1; cast to bool is purely for typing. + return bool(result) -def should_write_event_stats(event: Event | GroupEvent): +def should_write_event_stats(event: Event | GroupEvent) -> bool: # For now, we only want to write these stats for error events. If we start writing them for # other event types we'll throw off existing stats and potentially cause various alerts to fire. # We might decide to write these stats for other event types later, either under different keys # or with differentiating tags. - return ( + return bool( event.group and event.group.issue_category == GroupCategory.ERROR and event.group.platform is not None ) -def format_event_platform(event: Event | GroupEvent): +def format_event_platform(event: Event | GroupEvent) -> str | None: if not event.group: logger.error( "Group not found on event during formatting", extra={"event_id": event.event_id} ) - return + return None if not event.group.platform: logger.error( "Platform not found on group during formatting", extra={"event_id": event.event_id, "group_id": event.group.id}, ) - return + return None platform = event.group.platform return platform.split("-", 1)[0].split("_", 1)[0] @@ -152,7 +153,9 @@ def _capture_group_stats(job: PostProcessJob) -> None: @sentry_sdk.trace -def should_issue_owners_ratelimit(project_id: int, group_id: int, organization_id: int | None): +def should_issue_owners_ratelimit( + project_id: int, group_id: int, organization_id: int | None +) -> bool: """ Make sure that we do not accept more groups than the enforced_limit at the project level. """ @@ -182,7 +185,7 @@ def should_issue_owners_ratelimit(project_id: int, group_id: int, organization_i @metrics.wraps("post_process.handle_owner_assignment") @sentry_sdk.trace -def handle_owner_assignment(job): +def handle_owner_assignment(job: PostProcessJob) -> None: """ The handle_owner_assignment task attempts to find issue owners for a group. We call `ProjectOwnership.get_issue_owners` to find issue owners, and then @@ -295,7 +298,7 @@ def handle_owner_assignment(job): @sentry_sdk.trace -def handle_invalid_group_owners(group): +def handle_invalid_group_owners(group: Group) -> None: from sentry.models.groupowner import GroupOwner, GroupOwnerType invalid_group_owners = GroupOwner.objects.filter( @@ -315,7 +318,7 @@ def handle_group_owners( project: Project, group: Group, issue_owners: Sequence[tuple[Rule, Sequence[Team | RpcUser], str]], -): +) -> None: """ Stores group owners generated by `ProjectOwnership.get_issue_owners` in the `GroupOwner` model, and handles any diffing/changes of which owners we're keeping. @@ -343,7 +346,7 @@ def handle_group_owners( group=group, type__in=[GroupOwnerType.OWNERSHIP_RULE.value, GroupOwnerType.CODEOWNERS.value], ) - new_owners: dict = {} + new_owners: dict[tuple[type, int | None, str], list[Rule]] = {} for rule, owners, source in issue_owners: for owner in owners: # Can potentially have multiple rules pointing to the same owner @@ -435,7 +438,7 @@ def handle_group_owners( pass -def update_existing_attachments(job): +def update_existing_attachments(job: PostProcessJob) -> None: """ Attaches the group_id to all event attachments that were either: @@ -451,7 +454,7 @@ def update_existing_attachments(job): ) -def fetch_buffered_group_stats(group): +def fetch_buffered_group_stats(group: Group) -> None: """ Populates `times_seen_pending` with the number of buffered increments to `times_seen` for this group. `times_seen_with_pending` can subsequently be used as the total times seen, @@ -485,17 +488,17 @@ def should_retry_fetch(attempt: int, e: Exception) -> bool: silo_mode=SiloMode.CELL, ) def post_process_group( - is_new, - is_regression, - is_new_group_environment, - cache_key, - group_id=None, + is_new: bool, + is_regression: bool | None, + is_new_group_environment: bool, + cache_key: str | None, + group_id: int | None = None, occurrence_id: str | None = None, *, project_id: int, eventstream_type: str | None = None, - **kwargs, -): + **kwargs: Any, +) -> None: """ Fires post processing hooks for a group. """ @@ -513,6 +516,7 @@ def post_process_group( # We use the data being present/missing in the processing store # to ensure that we don't duplicate work should the forwarding consumers # need to rewind history. + assert cache_key is not None data = event_processing_store.get(cache_key) if not data: logger.info( @@ -603,7 +607,7 @@ def get_event_raise_exception() -> Event: group_state: GroupState = { "id": group_id, "is_new": is_new, - "is_regression": is_regression, + "is_regression": bool(is_regression), "is_new_group_environment": is_new_group_environment, } @@ -885,7 +889,7 @@ def process_snoozes(job: PostProcessJob) -> None: def process_replay_link(job: PostProcessJob) -> None: - def _get_replay_id(event): + def _get_replay_id(event: GroupEvent) -> str | None: # replay ids can either come as a context, or a tag. # right now they come as a context on non-js events, # and javascript transaction (through DSC context) @@ -1246,7 +1250,7 @@ def process_similarity(job: PostProcessJob) -> None: safe_execute(similarity.record, event.project, [event]) -def fire_error_processed(job: PostProcessJob): +def fire_error_processed(job: PostProcessJob) -> None: if job["is_reprocessed"]: return event = job["event"] @@ -1258,7 +1262,7 @@ def fire_error_processed(job: PostProcessJob): ) -def process_processing_errors_eap(job: PostProcessJob): +def process_processing_errors_eap(job: PostProcessJob) -> None: if job["is_reprocessed"]: return @@ -1276,13 +1280,13 @@ def process_processing_errors_eap(job: PostProcessJob): produce_processing_errors_to_eap(event.project, event.data, processing_errors) -def process_processing_issue_detection(job: PostProcessJob): +def process_processing_issue_detection(job: PostProcessJob) -> None: from sentry.processing_errors.detection import detect_processing_issues detect_processing_issues(job) -def sdk_crash_monitoring(job: PostProcessJob): +def sdk_crash_monitoring(job: PostProcessJob) -> None: from sentry.utils.sdk_crashes.sdk_crash_detection import sdk_crash_detection if job["is_reprocessed"]: @@ -1302,7 +1306,7 @@ def sdk_crash_monitoring(job: PostProcessJob): sdk_crash_detection.detect_sdk_crash(event=event, configs=configs) -def plugin_post_process_group(plugin_slug, event, **kwargs): +def plugin_post_process_group(plugin_slug: str, event: GroupEvent, **kwargs: Any) -> None: """ Fires post processing hooks for a group. """ @@ -1324,8 +1328,10 @@ def plugin_post_process_group(plugin_slug, event, **kwargs): logger.warning("post_process.process_error", extra={"exception": e}) -def feedback_filter_decorator(func): - def wrapper(job): +def feedback_filter_decorator( + func: Callable[[PostProcessJob], None], +) -> Callable[[PostProcessJob], None]: + def wrapper(job: PostProcessJob) -> None: if not should_postprocess_feedback(job): return return func(job) @@ -1425,7 +1431,7 @@ def link_event_to_user_report(job: PostProcessJob) -> None: MIN_EVENTS_FOR_NEW_ESCALATION = 10 -def detect_new_escalation(job: PostProcessJob): +def detect_new_escalation(job: PostProcessJob) -> None: """ Detects whether a new issue is escalating. New issues are issues less than MAX_NEW_ESCALATION_AGE_HOURS hours old. @@ -1497,7 +1503,7 @@ def detect_new_escalation(job: PostProcessJob): return -def detect_base_urls_for_uptime(job: PostProcessJob): +def detect_base_urls_for_uptime(job: PostProcessJob) -> None: from sentry.uptime.autodetect.detector import autodetect_base_url_for_project url = get_path(job["event"].data, "request", "url") @@ -1578,7 +1584,9 @@ def kick_off_seer_automation(job: PostProcessJob) -> None: ) -GROUP_CATEGORY_POST_PROCESS_PIPELINE = { +GROUP_CATEGORY_POST_PROCESS_PIPELINE: dict[ + GroupCategory, list[Callable[[PostProcessJob], None]] +] = { GroupCategory.ERROR: [ _capture_group_stats, process_snoozes, @@ -1620,7 +1628,7 @@ def kick_off_seer_automation(job: PostProcessJob) -> None: ], } -GENERIC_POST_PROCESS_PIPELINE = [ +GENERIC_POST_PROCESS_PIPELINE: list[Callable[[PostProcessJob], None]] = [ process_snoozes, process_inbox_adds, kick_off_seer_automation, diff --git a/src/sentry/testutils/cell.py b/src/sentry/testutils/cell.py index 1000e20161d8..0c5ca00508d0 100644 --- a/src/sentry/testutils/cell.py +++ b/src/sentry/testutils/cell.py @@ -53,10 +53,7 @@ def swap_state( monolith_cell = cells[0] with override_settings(SENTRY_MONOLITH_REGION=monolith_cell.name): if local_cell: - # TODO(cells): Remove SENTRY_REGION once all references in getsentry tests updated - with override_settings( - SENTRY_LOCAL_CELL=local_cell.name, SENTRY_REGION=local_cell.name - ): + with override_settings(SENTRY_LOCAL_CELL=local_cell.name): yield else: yield @@ -73,10 +70,7 @@ def swap_state( @contextmanager def swap_to_default_cell(self) -> Generator[None]: """Swap to the monolith cell when entering cell mode.""" - # TODO(cells): Remove SENTRY_REGION once all references in getsentry tests updated - with override_settings( - SENTRY_LOCAL_CELL=self._default_cell.name, SENTRY_REGION=self._default_cell.name - ): + with override_settings(SENTRY_LOCAL_CELL=self._default_cell.name): yield @contextmanager @@ -85,8 +79,7 @@ def swap_to_cell_by_name(self, cell_name: str) -> Generator[None]: cell = self.get_cell_by_name(cell_name) if cell is None: raise Exception("specified swap cell not found") - # TODO(cells): Remove SENTRY_REGION once all references in getsentry tests updated - with override_settings(SENTRY_LOCAL_CELL=cell.name, SENTRY_REGION=cell.name): + with override_settings(SENTRY_LOCAL_CELL=cell.name): yield diff --git a/src/sentry/testutils/pytest/sentry.py b/src/sentry/testutils/pytest/sentry.py index 4ee03479684b..ce5a965fc419 100644 --- a/src/sentry/testutils/pytest/sentry.py +++ b/src/sentry/testutils/pytest/sentry.py @@ -88,9 +88,6 @@ def _configure_test_env_cells() -> None: settings.SENTRY_LOCAL_CELL = cell_name settings.SENTRY_MONOLITH_REGION = cell_name - # TODO(cells): Remove once all references in getsentry tests updated - settings.SENTRY_REGION = cell_name - # This not only populates the environment with the default cell, but also # ensures that a TestEnvCellDirectory instance is injected into global state. # See sentry.testutils.cell.get_test_env_directory, which relies on it. diff --git a/src/sentry/utils/concurrent.py b/src/sentry/utils/concurrent.py index 033fc1c166b6..1cacad9a9306 100644 --- a/src/sentry/utils/concurrent.py +++ b/src/sentry/utils/concurrent.py @@ -19,27 +19,6 @@ logger = logging.getLogger(__name__) -def execute[T](function: Callable[..., T], daemon=True) -> Future[T]: - future: Future[T] = Future() - - def run(): - if not future.set_running_or_notify_cancel(): - return - - try: - result = function() - except Exception as e: - future.set_exception(e) - else: - future.set_result(result) - - t = threading.Thread(target=run) - t.daemon = daemon - t.start() - - return future - - @functools.total_ordering class PriorityTask[T](NamedTuple): priority: int diff --git a/src/sentry/viewer_context.py b/src/sentry/viewer_context.py index f1e6d3ee1cfe..b3c28788efc0 100644 --- a/src/sentry/viewer_context.py +++ b/src/sentry/viewer_context.py @@ -5,7 +5,7 @@ import dataclasses import enum from collections.abc import Generator -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any if TYPE_CHECKING: from sentry.auth.services.auth import AuthenticatedToken @@ -51,6 +51,17 @@ class ViewerContext: # NOT propagated across process/service boundaries. token: AuthenticatedToken | None = None + def serialize(self) -> dict[str, Any]: + """Serialize to a dict for cross-service headers (e.g. X-Viewer-Context).""" + result: dict[str, Any] = {"actor_type": self.actor_type.value} + if self.organization_id is not None: + result["organization_id"] = self.organization_id + if self.user_id is not None: + result["user_id"] = self.user_id + if self.token is not None: + result["token"] = {"kind": self.token.kind, "scopes": list(self.token.get_scopes())} + return result + @contextlib.contextmanager def viewer_context_scope(ctx: ViewerContext) -> Generator[None]: diff --git a/src/sentry/web/frontend/release_webhook.py b/src/sentry/web/frontend/release_webhook.py index 3b5cfc9a059e..e5d8093e9d76 100644 --- a/src/sentry/web/frontend/release_webhook.py +++ b/src/sentry/web/frontend/release_webhook.py @@ -109,6 +109,7 @@ def post(self, request: HttpRequest, plugin_id, project_id, signature) -> HttpRe return HttpResponse(status=403) cls = plugin.get_release_hook() + assert cls is not None hook = cls(project) try: hook.handle(request) diff --git a/static/app/components/commandPalette/__stories__/components.tsx b/static/app/components/commandPalette/__stories__/components.tsx index f1caaefdf824..217c74917e0d 100644 --- a/static/app/components/commandPalette/__stories__/components.tsx +++ b/static/app/components/commandPalette/__stories__/components.tsx @@ -2,18 +2,17 @@ import {useCallback} from 'react'; import {addSuccessMessage} from 'sentry/actionCreators/indicator'; import {CommandPaletteProvider} from 'sentry/components/commandPalette/context'; +import {useCommandPaletteActionsRegister} from 'sentry/components/commandPalette/context'; import type { CommandPaletteAction, - CommandPaletteActionCallbackWithKey, - CommandPaletteActionLinkWithKey, + CommandPaletteActionWithKey, } from 'sentry/components/commandPalette/types'; import {CommandPalette} from 'sentry/components/commandPalette/ui/commandPalette'; -import {useCommandPaletteActions} from 'sentry/components/commandPalette/useCommandPaletteActions'; import {normalizeUrl} from 'sentry/utils/url/normalizeUrl'; import {useNavigate} from 'sentry/utils/useNavigate'; export function RegisterActions({actions}: {actions: CommandPaletteAction[]}) { - useCommandPaletteActions(actions); + useCommandPaletteActionsRegister(actions); return null; } @@ -21,11 +20,13 @@ export function CommandPaletteDemo() { const navigate = useNavigate(); const handleAction = useCallback( - (action: CommandPaletteActionLinkWithKey | CommandPaletteActionCallbackWithKey) => { + (action: CommandPaletteActionWithKey) => { if ('to' in action) { navigate(normalizeUrl(action.to)); - } else { + } else if ('onAction' in action) { action.onAction(); + } else { + // @TODO: implement async actions } }, [navigate] diff --git a/static/app/components/commandPalette/context.tsx b/static/app/components/commandPalette/context.tsx index b631d8d79214..92c63ad162e0 100644 --- a/static/app/components/commandPalette/context.tsx +++ b/static/app/components/commandPalette/context.tsx @@ -1,15 +1,71 @@ -import {createContext, useCallback, useContext, useReducer} from 'react'; +import { + createContext, + useCallback, + useContext, + useEffect, + useMemo, + useReducer, +} from 'react'; +import {uuid4} from '@sentry/core'; +import {slugify} from 'sentry/utils/slugify'; import {unreachable} from 'sentry/utils/unreachable'; import {CommandPaletteStateProvider} from './ui/commandPaletteStateContext'; -import type {CommandPaletteActionWithKey} from './types'; +import type {CommandPaletteAction, CommandPaletteActionWithKey} from './types'; + +function addKeysToActions( + actions: CommandPaletteAction[] +): CommandPaletteActionWithKey[] { + return actions.map(action => { + const kind = 'actions' in action ? 'group' : 'to' in action ? 'navigate' : 'callback'; + const actionKey = `${kind}:${slugify(action.display.label)}:${uuid4()}`; + + if ('actions' in action) { + return { + ...action, + actions: addKeysToChildActions(actionKey, action.actions), + key: actionKey, + }; + } + + return { + ...action, + key: actionKey, + }; + }); +} + +function addKeysToChildActions( + parentKey: string, + actions: CommandPaletteAction[] +): CommandPaletteActionWithKey[] { + return actions.map(action => { + const actionKey = `${parentKey}::${'actions' in action ? 'group' : 'to' in action ? 'navigate' : 'callback'}:${slugify(action.display.label)}`; + + if ('actions' in action) { + return { + ...action, + actions: addKeysToChildActions(actionKey, action.actions), + key: actionKey, + }; + } + + return { + ...action, + key: actionKey, + }; + }); +} type CommandPaletteProviderProps = {children: React.ReactNode}; type CommandPaletteActions = CommandPaletteActionWithKey[]; type Unregister = () => void; -type CommandPaletteRegistration = (actions: CommandPaletteActionWithKey[]) => Unregister; +type CommandPaletteRegistration = { + dispatch: React.Dispatch; + registerActions: (actions: CommandPaletteAction[]) => Unregister; +}; type CommandPaletteActionReducerAction = | { @@ -25,7 +81,7 @@ const CommandPaletteRegistrationContext = createContext(null); const CommandPaletteActionsContext = createContext(null); -export function useCommandPaletteRegistration(): CommandPaletteRegistration { +function useCommandPaletteRegistration(): CommandPaletteRegistration { const ctx = useContext(CommandPaletteRegistrationContext); if (ctx === null) { throw new Error( @@ -65,6 +121,7 @@ function actionsReducer( return result; } case 'unregister': + // @TODO(Jonas): this needs to support deep unregistering of actions return state.filter(action => !reducerAction.keys.includes(action.key)); default: unreachable(type); @@ -76,20 +133,45 @@ export function CommandPaletteProvider({children}: CommandPaletteProviderProps) const [actions, dispatch] = useReducer(actionsReducer, []); const registerActions = useCallback( - (newActions: CommandPaletteActionWithKey[]) => { - dispatch({type: 'register', actions: newActions}); + (newActions: CommandPaletteAction[]) => { + const actionsWithKeys = addKeysToActions(newActions); + + dispatch({type: 'register', actions: actionsWithKeys}); return () => { - dispatch({type: 'unregister', keys: newActions.map(a => a.key)}); + dispatch({type: 'unregister', keys: actionsWithKeys.map(a => a.key)}); }; }, [dispatch] ); + const registerContext = useMemo( + () => ({registerActions, dispatch}), + [registerActions, dispatch] + ); return ( - + {children} ); } + +/** + * Use this hook inside your page or feature component to register contextual actions with the global command palette. + * Actions are registered on mount and automatically unregistered on unmount, so they only appear in the palette while + * your component is rendered. This is ideal for page‑specific shortcuts. + * + * There are a few different types of actions you can register: + * + * - **Navigation actions**: Provide a `to` destination to navigate to when selected. + * - **Callback actions**: Provide an `onAction` handler to execute when selected. + * - **Nested actions**: Provide an `actions: CommandPaletteAction[]` array on a parent item to show a second level. Selecting the parent reveals its children. + * + * See the CommandPaletteAction type for more details on configuration. + */ +export function useCommandPaletteActionsRegister(actions: CommandPaletteAction[]) { + const {registerActions} = useCommandPaletteRegistration(); + + useEffect(() => registerActions(actions), [actions, registerActions]); +} diff --git a/static/app/components/commandPalette/types.tsx b/static/app/components/commandPalette/types.tsx index 0e8d8e9fc88b..0005d50ce7b8 100644 --- a/static/app/components/commandPalette/types.tsx +++ b/static/app/components/commandPalette/types.tsx @@ -1,7 +1,9 @@ import type {ReactNode} from 'react'; import type {LocationDescriptor} from 'history'; -interface CommonCommandPaletteAction { +import type {UseQueryOptions} from 'sentry/utils/queryClient'; + +interface Action { display: { /** Primary text shown to the user */ label: string; @@ -14,12 +16,33 @@ interface CommonCommandPaletteAction { keywords?: string[]; } -export interface CommandPaletteActionLink extends CommonCommandPaletteAction { +/** + * Actions that can be returned from an async resource query. + * Async results cannot themselves carry a `resource` — chained async lookups + * are not supported. Use CommandPaletteAction for registering top-level actions. + */ +interface CommandPaletteAsyncResultGroup extends Action { + actions: CommandPaletteAsyncResult[]; +} + +export type CommandPaletteAsyncResult = + | CommandPaletteActionLink + | CommandPaletteActionCallback + | CommandPaletteAsyncResultGroup; + +export type CMDKQueryOptions = UseQueryOptions< + any, + Error, + CommandPaletteAsyncResult[], + any +>; + +export interface CommandPaletteActionLink extends Action { /** Navigate to a route when selected */ to: LocationDescriptor; } -export interface CommandPaletteActionCallback extends CommonCommandPaletteAction { +interface CommandPaletteActionCallback extends Action { /** * Execute a callback when the action is selected. * Use the `to` prop if you want to navigate to a route. @@ -27,33 +50,63 @@ export interface CommandPaletteActionCallback extends CommonCommandPaletteAction onAction: () => void; } +interface CommandPaletteAsyncAction extends Action { + /** + * Execute a callback when the action is selected. + * Use the `to` prop if you want to navigate to a route. + */ + resource: (query: string) => CMDKQueryOptions; +} + +interface CommandPaletteAsyncActionGroup extends Action { + actions: CommandPaletteAction[]; + resource: (query: string) => CMDKQueryOptions; +} + export type CommandPaletteAction = | CommandPaletteActionLink | CommandPaletteActionCallback - | CommandPaletteActionGroup; + | CommandPaletteActionGroup + | CommandPaletteAsyncAction + | CommandPaletteAsyncActionGroup; -export interface CommandPaletteActionGroup extends CommonCommandPaletteAction { +interface CommandPaletteActionGroup extends Action { /** Nested actions to show when this action is selected */ - actions: Array< - CommandPaletteActionLink | CommandPaletteActionCallback | CommandPaletteActionGroup - >; + actions: CommandPaletteAction[]; } // Internally, a key is added to the actions in order to track them for registration and selection. -export type CommandPaletteActionLinkWithKey = CommandPaletteActionLink & {key: string}; -export type CommandPaletteActionCallbackWithKey = CommandPaletteActionCallback & { +type CommandPaletteActionLinkWithKey = CommandPaletteActionLink & {key: string}; +type CommandPaletteActionCallbackWithKey = CommandPaletteActionCallback & { key: string; }; +type CommandPaletteAsyncActionWithKey = CommandPaletteAsyncAction & { + key: string; +}; +type CommandPaletteAsyncActionGroupWithKey = Omit< + CommandPaletteAsyncActionGroup, + 'actions' +> & { + actions: CommandPaletteActionWithKey[]; + key: string; +}; + export type CommandPaletteActionWithKey = + // Sync actions (to, callback, group) | CommandPaletteActionLinkWithKey | CommandPaletteActionCallbackWithKey - | CommandPaletteActionGroupWithKey; + | CommandPaletteActionGroupWithKey + // Async actions + | CommandPaletteAsyncActionWithKey + | CommandPaletteAsyncActionGroupWithKey; -export interface CommandPaletteActionGroupWithKey extends CommandPaletteActionGroup { +interface CommandPaletteActionGroupWithKey extends CommandPaletteActionGroup { actions: Array< | CommandPaletteActionLinkWithKey | CommandPaletteActionCallbackWithKey | CommandPaletteActionGroupWithKey + | CommandPaletteAsyncActionWithKey + | CommandPaletteAsyncActionGroupWithKey >; key: string; } diff --git a/static/app/components/commandPalette/ui/commandPalette.spec.tsx b/static/app/components/commandPalette/ui/commandPalette.spec.tsx index cc40ee749b76..3ed8dc327e93 100644 --- a/static/app/components/commandPalette/ui/commandPalette.spec.tsx +++ b/static/app/components/commandPalette/ui/commandPalette.spec.tsx @@ -2,6 +2,8 @@ import {useCallback} from 'react'; import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary'; +jest.unmock('lodash/debounce'); + jest.mock('@tanstack/react-virtual', () => ({ useVirtualizer: ({count}: {count: number}) => { const virtualItems = Array.from({length: count}, (_, index) => ({ @@ -23,17 +25,16 @@ jest.mock('@tanstack/react-virtual', () => ({ import {closeModal} from 'sentry/actionCreators/modal'; import * as modalActions from 'sentry/actionCreators/modal'; import {CommandPaletteProvider} from 'sentry/components/commandPalette/context'; +import {useCommandPaletteActionsRegister} from 'sentry/components/commandPalette/context'; import type { CommandPaletteAction, - CommandPaletteActionCallbackWithKey, - CommandPaletteActionLinkWithKey, + CommandPaletteActionWithKey, } from 'sentry/components/commandPalette/types'; import {CommandPalette} from 'sentry/components/commandPalette/ui/commandPalette'; -import {useCommandPaletteActions} from 'sentry/components/commandPalette/useCommandPaletteActions'; import {useNavigate} from 'sentry/utils/useNavigate'; function RegisterActions({actions}: {actions: CommandPaletteAction[]}) { - useCommandPaletteActions(actions); + useCommandPaletteActionsRegister(actions); return null; } @@ -47,11 +48,13 @@ function GlobalActionsComponent({ const navigate = useNavigate(); const handleAction = useCallback( - (action: CommandPaletteActionLinkWithKey | CommandPaletteActionCallbackWithKey) => { + (action: CommandPaletteActionWithKey) => { if ('to' in action) { navigate(action.to); - } else { + } else if ('onAction' in action) { action.onAction(); + } else { + // @TODO: implement async actions } closeModal(); }, @@ -224,6 +227,14 @@ describe('CommandPalette', () => { ).toBeInTheDocument(); }); + it('preserves spaces in typed query', async () => { + render(); + const input = await screen.findByRole('textbox', {name: 'Search commands'}); + await userEvent.type(input, 'test query'); + + expect(input).toHaveValue('test query'); + }); + it('search is case-insensitive', async () => { render(); const input = await screen.findByRole('textbox', {name: 'Search commands'}); diff --git a/static/app/components/commandPalette/ui/commandPalette.tsx b/static/app/components/commandPalette/ui/commandPalette.tsx index d1b484e0ba04..5b28ce086c66 100644 --- a/static/app/components/commandPalette/ui/commandPalette.tsx +++ b/static/app/components/commandPalette/ui/commandPalette.tsx @@ -21,9 +21,7 @@ import {Text} from '@sentry/scraps/text'; import {useCommandPaletteActions} from 'sentry/components/commandPalette/context'; import type { - CommandPaletteActionCallbackWithKey, - CommandPaletteActionGroupWithKey, - CommandPaletteActionLinkWithKey, + CMDKQueryOptions, CommandPaletteActionWithKey, } from 'sentry/components/commandPalette/types'; import { @@ -32,14 +30,18 @@ import { } from 'sentry/components/commandPalette/ui/commandPaletteStateContext'; import {useCommandPaletteAnalytics} from 'sentry/components/commandPalette/useCommandPaletteAnalytics'; import {FeedbackButton} from 'sentry/components/feedbackButton/feedbackButton'; +import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {IconArrow, IconClose, IconSearch} from 'sentry/icons'; import {IconDefaultsProvider} from 'sentry/icons/useIconDefaults'; import {t} from 'sentry/locale'; +import {useQueries} from 'sentry/utils/queryClient'; import {fzf} from 'sentry/utils/search/fzf'; import type {Theme} from 'sentry/utils/theme'; +import {useDebouncedValue} from 'sentry/utils/useDebouncedValue'; const MotionButton = motion.create(Button); const MotionIconSearch = motion.create(IconSearch); +const MotionContainer = motion.create(Container); function makeLeadingItemAnimation(theme: Theme) { return { @@ -65,9 +67,7 @@ type CommandPaletteActionWithListItemType = CommandPaletteActionWithKey & { }; interface CommandPaletteProps { - onAction: ( - action: CommandPaletteActionCallbackWithKey | CommandPaletteActionLinkWithKey - ) => void; + onAction: (action: CommandPaletteActionWithKey) => void; } export function CommandPalette(props: CommandPaletteProps) { @@ -83,8 +83,8 @@ export function CommandPalette(props: CommandPaletteProps) { preload(errorIllustration, {as: 'image'}); } - const actions = useMemo(() => { - const virtualRoot: CommandPaletteActionGroupWithKey = { + const root: CommandPaletteActionWithKey = useMemo(() => { + return { ...state.action?.value.action, key: 'virtual-root', actions: @@ -98,9 +98,42 @@ export function CommandPalette(props: CommandPaletteProps) { ...state.action?.value.action?.display, }, }; + }, [state.action, allActions]); + + const [resourceActions, queries] = useMemo(() => { + const actions = collectResourceActions(root, !!state.query); + return [actions, actions.map(({resource}) => resource(state.query))]; + }, [root, state.query]); + + const asyncQueries = useQueries({ + // Queries needs to be stable + queries, + }); + + const asyncChildrenMap = useMemo(() => { + if (asyncQueries.some(query => query.isFetching)) { + return new Map(); + } + const map = new Map(); + resourceActions.forEach(({key}, i) => { + const data = asyncQueries[i]?.data; + if (data?.length) { + map.set( + key, + data.map( + (action, j) => + ({...action, key: `${key}:async:${j}`}) as CommandPaletteActionWithKey + ) + ); + } + }); + return map; + }, [resourceActions, asyncQueries]); + + const actions = useMemo(() => { if (!state.query) { - return flattenActions(virtualRoot, null); + return flattenActions(root, null, asyncChildrenMap); } const scores = new Map< @@ -108,16 +141,11 @@ export function CommandPalette(props: CommandPaletteProps) { {action: CommandPaletteActionWithKey; score: {matched: boolean; score: number}} >(); - scoreTree(virtualRoot, scores, state.query.toLowerCase()); - return flattenActions(virtualRoot, scores); - }, [allActions, state.action, state.query]); + scoreTree(root, scores, state.query.toLowerCase(), asyncChildrenMap); + return flattenActions(root, scores, asyncChildrenMap); + }, [root, state.query, asyncChildrenMap]); - const filteredActionCount = useMemo( - () => actions.filter(a => a.listItemType === 'action').length, - [actions] - ); - - const analytics = useCommandPaletteAnalytics(filteredActionCount); + const analytics = useCommandPaletteAnalytics(actions.length); const sectionKeys = useMemo(() => { return new Set( @@ -150,6 +178,13 @@ export function CommandPalette(props: CommandPaletteProps) { ), + details: action.display.details ? ( + + + {action.display.details} + + + ) : undefined, hideCheck: true, children: [], }} @@ -203,6 +238,11 @@ export function CommandPalette(props: CommandPaletteProps) { keyboardDelegate: delegate, shouldFocusWrap: true, ref: state.input, + // Type-ahead is designed for navigating list items by typing — it intercepts + // Space (via onKeyDownCapture) when there is already a search term, which + // prevents the space from being inserted into the text input. Disable it + // here because filtering is handled by the input's own onChange instead. + disallowTypeAhead: true, }); const onActionSelection = useCallback( @@ -227,6 +267,12 @@ export function CommandPalette(props: CommandPaletteProps) { [actions, analytics, dispatch, props, treeState] ); + const debouncedQuery = useDebouncedValue(state.query, 300); + + const isLoading = + state.query.length > 0 && + (debouncedQuery !== state.query || asyncQueries.some(query => query.isFetching)); + return ( @@ -236,7 +282,18 @@ export function CommandPalette(props: CommandPaletteProps) { - {state.action ? ( + {isLoading ? ( + + + + ) : state.action ? ( {containerProps => ( - {treeState.collection.size === 0 ? ( + {treeState.collection.size === 0 && + // Don't show no results if we're still fetching data + asyncQueries.every(query => !query.isFetching) ? ( ) : ( @@ -354,6 +414,44 @@ export function CommandPalette(props: CommandPaletteProps) { ); } +function collectResourceActions( + root: CommandPaletteActionWithKey, + isSearching: boolean +): Array<{key: string; resource: (query: string) => CMDKQueryOptions}> { + const result: Array<{key: string; resource: (query: string) => CMDKQueryOptions}> = []; + + if (isSearching) { + function dfs(node: CommandPaletteActionWithKey) { + if ('resource' in node) { + result.push({key: node.key, resource: node.resource}); + } + if ('actions' in node) { + for (const child of node.actions) { + dfs(child); + } + } + } + dfs(root); + return result; + } + + // Browse mode mirrors the flattenActions no-query path: root's children + their children + for (const action of 'actions' in root ? root.actions : [root]) { + if ('resource' in action) { + result.push({key: action.key, resource: action.resource}); + } + if ('actions' in action) { + for (const child of action.actions) { + if ('resource' in child) { + result.push({key: child.key, resource: child.resource}); + } + } + } + } + + return result; +} + function score( query: string, action: CommandPaletteActionWithKey @@ -369,12 +467,13 @@ function score( } function scoreTree( - root: CommandPaletteActionGroupWithKey, + root: CommandPaletteActionWithKey, scores: Map< CommandPaletteActionWithKey['key'], {action: CommandPaletteActionWithKey; score: {matched: boolean; score: number}} >, - query: string + query: string, + asyncChildrenMap: Map ): void { function dfs(node: CommandPaletteActionWithKey) { if ('actions' in node) { @@ -383,6 +482,10 @@ function scoreTree( } } + for (const child of asyncChildrenMap.get(node.key) ?? []) { + dfs(child); + } + const scoreValue = score(query, node); if (scoreValue.matched) { scores.set(node.key, {action: node, score: scoreValue}); @@ -397,7 +500,8 @@ function flattenActions( scores: Map< CommandPaletteActionWithKey['key'], {action: CommandPaletteActionWithKey; score: {matched: boolean; score: number}} - > | null + > | null, + asyncChildrenMap: Map ): CommandPaletteActionWithListItemType[] { const results: CommandPaletteActionWithListItemType[] = []; @@ -408,12 +512,15 @@ function flattenActions( listItemType: 'actions' in action ? 'section' : 'action', }); + const asyncChildren = asyncChildrenMap.get(action.key) ?? []; + if ('actions' in action) { - for (const child of action.actions) { - results.push({ - ...child, - listItemType: 'action', - }); + for (const child of [...action.actions, ...asyncChildren]) { + results.push({...child, listItemType: 'action'}); + } + } else { + for (const child of asyncChildren) { + results.push({...child, listItemType: 'action'}); } } } @@ -432,6 +539,10 @@ function flattenActions( } else { groups.push({...node, listItemType: 'action'}); } + + for (const child of asyncChildrenMap.get(node.key) ?? []) { + dfs(child); + } } dfs(root); @@ -441,12 +552,20 @@ function flattenActions( let bScore = 0; if ('actions' in a) { aScore = Math.max( - ...a.actions.map(action => scores?.get(action.key)?.score.score ?? 0) + 0, + ...a.actions.map(action => scores?.get(action.key)?.score.score ?? 0), + ...(asyncChildrenMap.get(a.key) ?? []).map( + action => scores?.get(action.key)?.score.score ?? 0 + ) ); } if ('actions' in b) { bScore = Math.max( - ...b.actions.map(action => scores?.get(action.key)?.score.score ?? 0) + 0, + ...b.actions.map(action => scores?.get(action.key)?.score.score ?? 0), + ...(asyncChildrenMap.get(b.key) ?? []).map( + action => scores?.get(action.key)?.score.score ?? 0 + ) ); } return bScore - aScore; @@ -457,11 +576,15 @@ function flattenActions( return []; } if ('actions' in result) { - const resultActions = result.actions.filter( + const matchedStaticChildren = result.actions.filter( + action => scores?.get(action.key)?.score.matched + ); + const matchedAsyncChildren = (asyncChildrenMap.get(result.key) ?? []).filter( action => scores?.get(action.key)?.score.matched ); + const allMatchedChildren = [...matchedStaticChildren, ...matchedAsyncChildren]; - if (!resultActions.length) { + if (!allMatchedChildren.length) { return []; } @@ -471,7 +594,7 @@ function flattenActions( // React duplicate-key error (both entries would otherwise share the // same key). {...result, key: `${result.key}:header`, listItemType: 'section'}, - ...resultActions + ...allMatchedChildren .sort((a, b) => { if (!a || !b) { return 0; diff --git a/static/app/components/commandPalette/ui/modal.tsx b/static/app/components/commandPalette/ui/modal.tsx index 27955990c254..fd0358a2449e 100644 --- a/static/app/components/commandPalette/ui/modal.tsx +++ b/static/app/components/commandPalette/ui/modal.tsx @@ -3,29 +3,23 @@ import {css} from '@emotion/react'; import type {ModalRenderProps} from 'sentry/actionCreators/modal'; import {closeModal} from 'sentry/actionCreators/modal'; -import type { - CommandPaletteActionCallbackWithKey, - CommandPaletteActionLinkWithKey, -} from 'sentry/components/commandPalette/types'; +import type {CommandPaletteActionWithKey} from 'sentry/components/commandPalette/types'; import {CommandPalette} from 'sentry/components/commandPalette/ui/commandPalette'; -import {useCommandPaletteState} from 'sentry/components/commandPalette/ui/commandPaletteStateContext'; -import {useDsnLookupActions} from 'sentry/components/commandPalette/useDsnLookupActions'; import type {Theme} from 'sentry/utils/theme'; import {normalizeUrl} from 'sentry/utils/url/normalizeUrl'; import {useNavigate} from 'sentry/utils/useNavigate'; export default function CommandPaletteModal({Body}: ModalRenderProps) { const navigate = useNavigate(); - const {query} = useCommandPaletteState(); - - useDsnLookupActions(query); const handleSelect = useCallback( - (action: CommandPaletteActionLinkWithKey | CommandPaletteActionCallbackWithKey) => { + (action: CommandPaletteActionWithKey) => { if ('to' in action) { navigate(normalizeUrl(action.to)); - } else { + } else if ('onAction' in action) { action.onAction(); + } else { + // @TODO: handle async actions } closeModal(); }, diff --git a/static/app/components/commandPalette/useCommandPaletteActions.mdx b/static/app/components/commandPalette/useCommandPaletteActions.mdx index 48b4704662ce..342234b13607 100644 --- a/static/app/components/commandPalette/useCommandPaletteActions.mdx +++ b/static/app/components/commandPalette/useCommandPaletteActions.mdx @@ -13,7 +13,6 @@ import {addSuccessMessage} from 'sentry/actionCreators/indicator'; import {toggleCommandPalette} from 'sentry/actionCreators/modal'; import {CommandPaletteProvider} from 'sentry/components/commandPalette/context'; -import {useCommandPaletteActions} from 'sentry/components/commandPalette/useCommandPaletteActions'; import * as Storybook from 'sentry/stories'; diff --git a/static/app/components/commandPalette/useCommandPaletteActions.tsx b/static/app/components/commandPalette/useCommandPaletteActions.tsx deleted file mode 100644 index b772210201b3..000000000000 --- a/static/app/components/commandPalette/useCommandPaletteActions.tsx +++ /dev/null @@ -1,84 +0,0 @@ -import {useEffect, useId} from 'react'; - -import {slugify} from 'sentry/utils/slugify'; - -import {useCommandPaletteRegistration} from './context'; -import type { - CommandPaletteAction, - CommandPaletteActionCallbackWithKey, - CommandPaletteActionLinkWithKey, - CommandPaletteActionWithKey, - CommandPaletteActionGroupWithKey, -} from './types'; - -function addKeysToActions( - id: string, - actions: CommandPaletteAction[] -): CommandPaletteActionWithKey[] { - return actions.map(action => { - const kind = 'actions' in action ? 'group' : 'to' in action ? 'navigate' : 'callback'; - const actionKey = `${id}:${kind}:${slugify(action.display.label)}`; - - if ('actions' in action) { - return { - ...action, - actions: addKeysToChildActions(actionKey, action.actions), - key: actionKey, - }; - } - - return { - ...action, - key: actionKey, - }; - }); -} - -function addKeysToChildActions( - parentKey: string, - actions: CommandPaletteAction[] -): Array< - | CommandPaletteActionLinkWithKey - | CommandPaletteActionCallbackWithKey - | CommandPaletteActionGroupWithKey -> { - return actions.map(action => { - const actionKey = `${parentKey}::${'actions' in action ? 'group' : 'to' in action ? 'navigate' : 'callback'}:${slugify(action.display.label)}`; - - if ('actions' in action) { - return { - ...action, - actions: addKeysToChildActions(actionKey, action.actions), - key: actionKey, - }; - } - - return { - ...action, - key: actionKey, - }; - }); -} - -/** - * Use this hook inside your page or feature component to register contextual actions with the global command palette. - * Actions are registered on mount and automatically unregistered on unmount, so they only appear in the palette while - * your component is rendered. This is ideal for page‑specific shortcuts. - * - * There are a few different types of actions you can register: - * - * - **Navigation actions**: Provide a `to` destination to navigate to when selected. - * - **Callback actions**: Provide an `onAction` handler to execute when selected. - * - **Nested actions**: Provide an `actions: CommandPaletteAction[]` array on a parent item to show a second level. Selecting the parent reveals its children. - * - * See the CommandPaletteAction type for more details on configuration. - */ -export function useCommandPaletteActions(actions: CommandPaletteAction[]) { - const id = useId(); - const registerActions = useCommandPaletteRegistration(); - - useEffect( - () => registerActions(addKeysToActions(id, actions)), - [actions, id, registerActions] - ); -} diff --git a/static/app/components/commandPalette/useCommandPaletteAnalytics.tsx b/static/app/components/commandPalette/useCommandPaletteAnalytics.tsx index 352c13c2ca70..44ad6b175064 100644 --- a/static/app/components/commandPalette/useCommandPaletteAnalytics.tsx +++ b/static/app/components/commandPalette/useCommandPaletteAnalytics.tsx @@ -2,11 +2,7 @@ import {useEffect, useMemo, useRef} from 'react'; import * as Sentry from '@sentry/react'; import uniqueId from 'lodash/uniqueId'; -import type { - CommandPaletteActionCallbackWithKey, - CommandPaletteActionLinkWithKey, - CommandPaletteActionWithKey, -} from 'sentry/components/commandPalette/types'; +import type {CommandPaletteActionWithKey} from 'sentry/components/commandPalette/types'; import { getActionPath, type LinkedList, @@ -36,7 +32,7 @@ function getLinkedListDepth(node: LinkedList | null): number { */ export function useCommandPaletteAnalytics(filteredActionCount: number): { recordAction: ( - action: CommandPaletteActionLinkWithKey | CommandPaletteActionCallbackWithKey, + action: CommandPaletteActionWithKey, resultIndex: number, group: string ) => void; @@ -140,7 +136,7 @@ export function useCommandPaletteAnalytics(filteredActionCount: number): { return useMemo( () => ({ recordAction( - action: CommandPaletteActionLinkWithKey | CommandPaletteActionCallbackWithKey, + action: CommandPaletteActionWithKey, resultIndex: number, group: string ) { diff --git a/static/app/components/commandPalette/useDsnLookupActions.spec.tsx b/static/app/components/commandPalette/useDsnLookupActions.spec.tsx deleted file mode 100644 index 23a15d1cb81f..000000000000 --- a/static/app/components/commandPalette/useDsnLookupActions.spec.tsx +++ /dev/null @@ -1,115 +0,0 @@ -import {OrganizationFixture} from 'sentry-fixture/organization'; - -import {render, screen, waitFor} from 'sentry-test/reactTestingLibrary'; - -import { - CommandPaletteProvider, - useCommandPaletteActions as useRegisteredActions, -} from 'sentry/components/commandPalette/context'; -import {useDsnLookupActions} from 'sentry/components/commandPalette/useDsnLookupActions'; - -const org = OrganizationFixture({features: ['cmd-k-dsn-lookup']}); - -function DsnLookupHarness({query}: {query: string}) { - useDsnLookupActions(query); - const actions = useRegisteredActions(); - - return ( -
    - {actions.map(action => ( -
  • - {action.display.label} -
    • - ))} -
- ); -} - -function renderWithProvider(query: string) { - return render( - - - , - {organization: org} - ); -} - -describe('useDsnLookupActions', () => { - beforeEach(() => { - MockApiClient.clearMockResponses(); - }); - - it('returns actions for a valid DSN', async () => { - const dsn = 'https://abc123def456abc123def456abc123de@o1.ingest.sentry.io/123'; - MockApiClient.addMockResponse({ - url: `/organizations/${org.slug}/dsn-lookup/`, - body: { - organizationSlug: 'test-org', - projectSlug: 'test-project', - projectId: '42', - projectName: 'Test Project', - projectPlatform: 'javascript', - keyLabel: 'Default', - keyId: '1', - }, - match: [MockApiClient.matchQuery({dsn})], - }); - - renderWithProvider(dsn); - - await waitFor(() => { - expect(screen.getAllByRole('listitem').length).toBeGreaterThan(0); - }); - - const items = screen.getAllByRole('listitem'); - expect(items).toHaveLength(3); - - expect(items[0]).toHaveAttribute('data-type', 'navigate'); - expect(items[0]).toHaveAttribute( - 'data-to', - '/organizations/test-org/issues/?project=42' - ); - - expect(items[1]).toHaveAttribute('data-type', 'navigate'); - expect(items[1]).toHaveAttribute( - 'data-to', - '/settings/test-org/projects/test-project/' - ); - - expect(items[2]).toHaveAttribute('data-type', 'navigate'); - expect(items[2]).toHaveAttribute( - 'data-to', - '/settings/test-org/projects/test-project/keys/' - ); - }); - - it('returns empty array for non-DSN query', () => { - const mockApi = MockApiClient.addMockResponse({ - url: `/organizations/${org.slug}/dsn-lookup/`, - body: {}, - }); - - renderWithProvider('some random text'); - - expect(screen.queryAllByRole('listitem')).toHaveLength(0); - expect(mockApi).not.toHaveBeenCalled(); - }); - - it('returns empty array for empty query', () => { - const mockApi = MockApiClient.addMockResponse({ - url: `/organizations/${org.slug}/dsn-lookup/`, - body: {}, - }); - - renderWithProvider(''); - - expect(screen.queryAllByRole('listitem')).toHaveLength(0); - expect(mockApi).not.toHaveBeenCalled(); - }); -}); diff --git a/static/app/components/commandPalette/useDsnLookupActions.tsx b/static/app/components/commandPalette/useDsnLookupActions.tsx deleted file mode 100644 index e598c8e4622c..000000000000 --- a/static/app/components/commandPalette/useDsnLookupActions.tsx +++ /dev/null @@ -1,56 +0,0 @@ -import {useMemo} from 'react'; - -import type {CommandPaletteAction} from 'sentry/components/commandPalette/types'; -import {useCommandPaletteActions} from 'sentry/components/commandPalette/useCommandPaletteActions'; -import { - DSN_PATTERN, - getDsnNavTargets, -} from 'sentry/components/search/sources/dsnLookupUtils'; -import type {DsnLookupResponse} from 'sentry/components/search/sources/dsnLookupUtils'; -import {IconIssues, IconList, IconSettings} from 'sentry/icons'; -import {getApiUrl} from 'sentry/utils/api/getApiUrl'; -import {useApiQuery} from 'sentry/utils/queryClient'; -import {useOrganization} from 'sentry/utils/useOrganization'; - -const ICONS: React.ReactElement[] = [ - , - , - , -]; - -export function useDsnLookupActions(query: string): void { - const organization = useOrganization({allowNull: true}); - const hasDsnLookup = organization?.features?.includes('cmd-k-dsn-lookup') ?? false; - const isDsn = DSN_PATTERN.test(query); - - const {data} = useApiQuery( - [ - getApiUrl('/organizations/$organizationIdOrSlug/dsn-lookup/', { - path: {organizationIdOrSlug: organization?.slug ?? ''}, - }), - {query: {dsn: query}}, - ], - { - staleTime: 30_000, - enabled: isDsn && !!organization && hasDsnLookup, - } - ); - - const actions: CommandPaletteAction[] = useMemo(() => { - if (!isDsn || !data) { - return []; - } - - return getDsnNavTargets(data).map((target, i) => ({ - to: target.to, - display: { - label: target.label, - details: target.description, - icon: ICONS[i], - }, - groupingKey: 'search-result' as const, - })); - }, [isDsn, data]); - - useCommandPaletteActions(actions); -} diff --git a/static/app/components/commandPalette/useGlobalCommandPaletteActions.tsx b/static/app/components/commandPalette/useGlobalCommandPaletteActions.tsx index 87b197e37fc8..ac5d3a540633 100644 --- a/static/app/components/commandPalette/useGlobalCommandPaletteActions.tsx +++ b/static/app/components/commandPalette/useGlobalCommandPaletteActions.tsx @@ -1,25 +1,43 @@ +import {useState} from 'react'; +import {SentryGlobalSearch} from '@sentry-internal/global-search'; +import DOMPurify from 'dompurify'; + import {ProjectAvatar} from '@sentry/scraps/avatar'; import {addLoadingMessage, addSuccessMessage} from 'sentry/actionCreators/indicator'; import {openInviteMembersModal} from 'sentry/actionCreators/modal'; -import type {CommandPaletteAction} from 'sentry/components/commandPalette/types'; -import {useCommandPaletteActions} from 'sentry/components/commandPalette/useCommandPaletteActions'; +import {useCommandPaletteActionsRegister} from 'sentry/components/commandPalette/context'; +import type { + CMDKQueryOptions, + CommandPaletteAction, + CommandPaletteAsyncResult, +} from 'sentry/components/commandPalette/types'; +import { + DSN_PATTERN, + getDsnNavTargets, +} from 'sentry/components/search/sources/dsnLookupUtils'; +import type {DsnLookupResponse} from 'sentry/components/search/sources/dsnLookupUtils'; import { IconAdd, IconCompass, IconDashboard, IconDiscord, IconDocs, + IconSearch, IconGithub, IconGraph, IconIssues, + IconList, IconOpen, IconSettings, IconStar, IconUser, IconPanel, + IconLock, } from 'sentry/icons'; import {t} from 'sentry/locale'; +import {apiOptions} from 'sentry/utils/api/apiOptions'; +import {queryOptions} from 'sentry/utils/queryClient'; import {useMutateUserOptions} from 'sentry/utils/useMutateUserOptions'; import {useOrganization} from 'sentry/utils/useOrganization'; import {useProjects} from 'sentry/utils/useProjects'; @@ -34,6 +52,12 @@ import {useStarredIssueViews} from 'sentry/views/navigation/secondary/sections/i import {useSecondaryNavigation} from 'sentry/views/navigation/secondaryNavigationContext'; import {getUserOrgNavigationConfiguration} from 'sentry/views/settings/organization/userOrgNavigationConfiguration'; +const DSN_ICONS: React.ReactElement[] = [ + , + , + , +]; + // This hook generates actions for all pages in the primary and secondary navigation. // TODO: Consider refactoring the navigation so that this can read from the same source // of truth and avoid divergence. @@ -296,13 +320,17 @@ function useNavigationToggleCollapsed(): CommandPaletteAction { */ export function useGlobalCommandPaletteActions() { const organization = useOrganization(); + const hasDsnLookup = organization.features.includes('cmd-k-dsn-lookup'); + const {projects} = useProjects(); const navigateActions = useNavigationActions(); const {mutateAsync: mutateUserOptions} = useMutateUserOptions(); const navigationToggleAction = useNavigationToggleCollapsed(); + const [search] = useState(() => new SentryGlobalSearch(['docs', 'develop'])); + const navPrefix = `/organizations/${organization.slug}`; - useCommandPaletteActions([ + useCommandPaletteActionsRegister([ ...navigateActions, // BEGIN ADD ACTIONS { @@ -315,6 +343,7 @@ export function useGlobalCommandPaletteActions() { label: t('Create Dashboard'), icon: , }, + keywords: [t('add dashboard')], to: `${navPrefix}/dashboards/new/`, }, { @@ -322,6 +351,7 @@ export function useGlobalCommandPaletteActions() { label: t('Create Alert'), icon: , }, + keywords: [t('add alert')], to: `${navPrefix}/issues/alerts/wizard/`, }, { @@ -329,6 +359,7 @@ export function useGlobalCommandPaletteActions() { label: t('Create Project'), icon: , }, + keywords: [t('add project')], to: `${navPrefix}/projects/new/`, }, { @@ -336,11 +367,70 @@ export function useGlobalCommandPaletteActions() { label: t('Invite Members'), icon: , }, - onAction: () => openInviteMembersModal(), + keywords: [t('team invite')], + onAction: openInviteMembersModal, }, ], }, // END ADD + // BEGIN DSN LOOKUP + { + display: {label: t('DSN')}, + keywords: [t('client keys')], + actions: [ + { + display: { + label: t('Project DSN Keys'), + icon: , + }, + keywords: [t('client keys'), t('dsn keys')], + actions: projects.map(project => ({ + display: { + label: project.name, + icon: , + }, + keywords: [`dsn ${project.name}`, `dsn ${project.slug}`], + to: `/settings/${organization.slug}/projects/${project.slug}/keys/`, + })), + }, + hasDsnLookup + ? { + display: { + label: t('Reverse DSN lookup'), + details: t( + 'Paste a DSN into the search bar to find the project it belongs to.' + ), + icon: , + }, + actions: [], + resource: (query: string): CMDKQueryOptions => { + return queryOptions({ + ...apiOptions.as()( + '/organizations/$organizationIdOrSlug/dsn-lookup/', + { + path: {organizationIdOrSlug: organization.slug}, + query: {dsn: query}, + staleTime: 30_000, + } + ), + enabled: DSN_PATTERN.test(query), + select: data => + getDsnNavTargets(data.json).map((target, i) => ({ + to: target.to, + display: { + label: target.label, + details: target.description, + icon: DSN_ICONS[i], + }, + keywords: [query], + })), + }); + }, + } + : undefined, + ].filter(action => action !== undefined), + }, + // END DSN LOOKUP // BEGIN HELP ACTIONS { display: { @@ -379,8 +469,52 @@ export function useGlobalCommandPaletteActions() { window.open('https://sentry.io/changelog/', '_blank', 'noreferrer'), }, ], + resource: (query: string) => { + return queryOptions({ + queryKey: ['command-palette-help-search', query, search], + queryFn: () => { + return search.query( + query, + { + searchAllIndexes: true, + }, + { + analyticsTags: ['source:command-palette'], + } + ); + }, + select: data => { + const actions: CommandPaletteAsyncResult[] = []; + + for (const index of data) { + // We'll limit async results to avoid overwhelming the UI + for (const hit of index.hits.slice(0, 3)) { + actions.push({ + display: { + label: DOMPurify.sanitize(hit.title ?? '', {ALLOWED_TAGS: []}), + details: DOMPurify.sanitize( + hit.context?.context1 ?? hit.context?.context2 ?? '', + {ALLOWED_TAGS: []} + ), + icon: , + }, + keywords: [hit.context?.context1, hit.context?.context2].filter( + value => value !== undefined && typeof value === 'string' + ), + onAction: () => { + window.open(hit.url, '_blank', 'noreferrer'); + }, + }); + } + } + + return actions; + }, + }); + }, }, // END HELP ACTIONS + // START UI ACTIONS { display: { label: t('Interface'), @@ -427,5 +561,6 @@ export function useGlobalCommandPaletteActions() { }, ], }, + // END UI ACTIONS ]); } diff --git a/static/app/components/core/slot/index.tsx b/static/app/components/core/slot/index.tsx new file mode 100644 index 000000000000..c270ab4e2a50 --- /dev/null +++ b/static/app/components/core/slot/index.tsx @@ -0,0 +1 @@ +export {slot, withSlots} from './slot'; diff --git a/static/app/components/core/slot/slot.spec.tsx b/static/app/components/core/slot/slot.spec.tsx new file mode 100644 index 000000000000..d23b461d357c --- /dev/null +++ b/static/app/components/core/slot/slot.spec.tsx @@ -0,0 +1,313 @@ +import {render, screen} from 'sentry-test/reactTestingLibrary'; + +import {slot, withSlots} from './'; + +describe('slot', () => { + it('returns a module with Provider, Outlet, and Fallback sub-components', () => { + const SlotModule = slot(['header', 'footer'] as const); + expect(SlotModule).toBeDefined(); + expect(SlotModule.Provider).toBeDefined(); + expect(SlotModule.Outlet).toBeDefined(); + expect(SlotModule.Fallback).toBeDefined(); + }); + + it('renders children in place when no Outlet is registered', () => { + const SlotModule = slot(['header'] as const); + + render( + + + inline content + + + ); + + expect(screen.getByText('inline content')).toBeInTheDocument(); + }); + + it('portals children to the Outlet element', () => { + const SlotModule = slot(['content'] as const); + + render( + + + {props =>
} + + + portaled content + + + ); + + expect(screen.getByTestId('slot-target')).toContainHTML( + 'portaled content' + ); + }); + + it('multiple slot consumers render their children independently', () => { + const SlotModule = slot(['a', 'b'] as const); + + render( + + + slot a content + + + slot b content + + + ); + + expect(screen.getByText('slot a content')).toBeInTheDocument(); + expect(screen.getByText('slot b content')).toBeInTheDocument(); + }); + + it('consumer throws when rendered outside provider', () => { + const SlotModule = slot(['nav'] as const); + + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); + + expect(() => + render( + + content + + ) + ).toThrow('SlotContext not found'); + + consoleError.mockRestore(); + }); + + it('Outlet throws when rendered outside provider', () => { + const SlotModule = slot(['aside'] as const); + + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); + + expect(() => + render( + {props =>
} + ) + ).toThrow('SlotContext not found'); + + consoleError.mockRestore(); + }); + + it('Outlet renders the element returned by the render prop', () => { + const SlotModule = slot(['sidebar'] as const); + + render( + + + {props =>
} + + + ); + + expect(screen.getByTestId('sidebar-root')).toBeInTheDocument(); + }); + + it('Outlet registers and unregisters the element on mount/unmount', () => { + const SlotModule = slot(['panel'] as const); + + const {unmount} = render( + + + {props =>
} + + + ); + + expect(screen.getByTestId('panel-root')).toBeInTheDocument(); + expect(() => unmount()).not.toThrow(); + }); + + it('provider renders its children', () => { + const SlotModule = slot(['x'] as const); + + render( + + child content + + ); + + expect(screen.getByText('child content')).toBeInTheDocument(); + }); + + it('separate slot() calls create independent slot systems', () => { + const SlotModule1 = slot(['zone'] as const); + const SlotModule2 = slot(['zone'] as const); + + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); + + expect(() => + render( + + + content + + + ) + ).toThrow('SlotContext not found'); + + consoleError.mockRestore(); + }); + + describe('Fallback', () => { + it('renders children into Outlet when no consumer is mounted', () => { + const SlotModule = slot(['feedback'] as const); + + render( + + + {props => ( +
+ + default feedback + +
+ )} + + + ); + + expect(screen.getByTestId('feedback-root')).toContainHTML( + 'default feedback' + ); + }); + + it('does not render when a consumer is mounted', () => { + const SlotModule = slot(['feedback'] as const); + + render( + + + {props => ( +
+ + default feedback + +
+ )} + + + custom feedback + + + ); + + expect(screen.queryByText('default feedback')).not.toBeInTheDocument(); + expect(screen.getByTestId('feedback-root')).toContainHTML( + 'custom feedback' + ); + }); + + it('throws when rendered outside provider', () => { + const SlotModule = slot(['x'] as const); + + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); + + expect(() => + render( + + fallback + + ) + ).toThrow('SlotContext not found'); + + consoleError.mockRestore(); + }); + + it('throws when rendered outside Outlet', () => { + const SlotModule = slot(['x'] as const); + + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); + + expect(() => + render( + + + fallback + + + ) + ).toThrow('Slot.Fallback must be rendered inside Slot.Outlet'); + + consoleError.mockRestore(); + }); + }); + + describe('Outlet ref stability', () => { + it('ref callback passed to render prop is stable across re-renders', () => { + const SlotModule = slot(['menu'] as const); + const refs: Array> = []; + + function TestComponent() { + return ( + + {props => { + refs.push(props.ref); + return
; + }} + + ); + } + + const {rerender} = render( + + + + ); + + rerender( + + + + ); + + expect(refs[0]).toBe(refs[refs.length - 1]); + }); + }); + + describe('withSlots', () => { + it('attaches a Slot property to a component', () => { + const SlotModule = slot(['header'] as const); + + function MyComponent() { + return
; + } + + const WithSlots = withSlots(MyComponent, SlotModule); + + expect(WithSlots.Slot).toBe(SlotModule); + }); + + it('renders the wrapped component and allows slot injection', () => { + const SlotModule = slot(['title'] as const); + + function MyComponent() { + return ( +
+ + {props => } + +
+ ); + } + + const WithSlots = withSlots(MyComponent, SlotModule); + + render( + + + + injected title + + + ); + + expect(screen.getByTestId('title-outlet')).toContainHTML( + 'injected title' + ); + }); + }); +}); diff --git a/static/app/components/core/slot/slot.tsx b/static/app/components/core/slot/slot.tsx new file mode 100644 index 000000000000..7c0aab1589a2 --- /dev/null +++ b/static/app/components/core/slot/slot.tsx @@ -0,0 +1,260 @@ +import { + createContext, + useCallback, + useContext, + useLayoutEffect, + useMemo, + useReducer, +} from 'react'; +import {createPortal} from 'react-dom'; + +type Slot = string; +type SlotValue = {counter: number; element: HTMLElement | null}; + +type SlotReducerState = Partial>; +type SlotReducerAction = + | { + name: T; + type: 'increment counter'; + } + | { + name: T; + type: 'decrement counter'; + } + | { + element: HTMLElement | null; + name: T; + type: 'register'; + } + | { + name: T; + type: 'unregister'; + }; + +type SlotReducer = React.Reducer< + SlotReducerState, + SlotReducerAction +>; + +type SlotContextValue = [ + SlotReducerState, + React.Dispatch>, +]; + +function makeSlotReducer(): SlotReducer { + return function reducer( + state: SlotReducerState, + action: SlotReducerAction + ): SlotReducerState { + switch (action.type) { + case 'increment counter': { + const currentSlot = state[action.name]; + return { + ...state, + [action.name]: { + element: currentSlot?.element ?? null, + counter: (currentSlot?.counter ?? 0) + 1, + }, + }; + } + case 'decrement counter': { + const currentSlot = state[action.name]; + if (!currentSlot) { + return state; + } + return { + ...state, + [action.name]: { + ...currentSlot, + counter: (currentSlot?.counter ?? 0) - 1, + }, + }; + } + case 'register': + return { + ...state, + [action.name]: { + counter: state[action.name]?.counter ?? 0, + element: action.element, + }, + }; + case 'unregister': { + const currentSlot = state[action.name]; + if (!currentSlot) { + return state; + } + return { + ...state, + [action.name]: { + counter: currentSlot?.counter ?? 0, + element: null, + }, + }; + } + default: + return state; + } + }; +} + +interface SlotProviderProps { + children: React.ReactNode; +} + +interface SlotConsumerProps { + children: React.ReactNode; + name: T; +} + +interface SlotOutletProps { + children: (props: {ref: React.RefCallback}) => React.ReactNode; + name: T; +} + +interface SlotFallbackProps { + children: React.ReactNode; +} + +type SlotModule = React.FunctionComponent> & { + Fallback: React.ComponentType; + Outlet: React.ComponentType>; + Provider: React.ComponentType; +}; + +function makeSlotConsumer( + context: React.Context | null> +) { + function SlotConsumer(props: SlotConsumerProps): React.ReactNode { + const ctx = useContext(context); + if (!ctx) { + throw new Error('SlotContext not found'); + } + + const [state, dispatch] = ctx; + const {name} = props; + useLayoutEffect(() => { + dispatch({type: 'increment counter', name}); + return () => dispatch({type: 'decrement counter', name}); + }, [dispatch, name]); + + const element = state[name]?.element; + if (!element) { + // Render in place as a fallback when no target element is registered yet + return props.children; + } + return createPortal(props.children, element); + } + + SlotConsumer.displayName = 'Slot.Consumer'; + return SlotConsumer; +} + +function makeSlotOutlet( + context: React.Context | null>, + outletNameContext: React.Context +) { + function SlotOutlet(props: SlotOutletProps): React.ReactNode { + const ctx = useContext(context); + + if (!ctx) { + throw new Error('SlotContext not found'); + } + + const [, dispatch] = ctx; + const {name} = props; + const ref = useCallback( + (element: HTMLElement | null) => { + if (!element) { + dispatch({type: 'unregister', name}); + return; + } + dispatch({type: 'register', name, element}); + }, + [dispatch, name] + ); + + return ( + + {props.children({ref})} + + ); + } + + SlotOutlet.displayName = 'Slot.Outlet'; + return SlotOutlet; +} + +function makeSlotFallback( + context: React.Context | null>, + outletNameContext: React.Context +) { + function SlotFallback({children}: SlotFallbackProps): React.ReactNode { + const ctx = useContext(context); + if (!ctx) { + throw new Error('SlotContext not found'); + } + + const name = useContext(outletNameContext); + if (name === null) { + throw new Error('Slot.Fallback must be rendered inside Slot.Outlet'); + } + + const [state] = ctx; + if ((state[name]?.counter ?? 0) > 0 || !state[name]?.element) { + return null; + } + + return createPortal(children, state[name].element); + } + + SlotFallback.displayName = 'Slot.Fallback'; + return SlotFallback; +} + +function makeSlotProvider( + context: React.Context | null> +): (props: SlotProviderProps) => React.ReactNode { + const reducer = makeSlotReducer(); + + function SlotProvider({children}: SlotProviderProps) { + const [value, dispatch] = useReducer(reducer, {}); + + const contextValue = useMemo( + () => [value, dispatch] satisfies SlotContextValue, + [value, dispatch] + ); + return {children}; + } + + SlotProvider.displayName = `Slot.Provider`; + return SlotProvider as (props: SlotProviderProps) => React.ReactNode; +} + +export function slot(names: T): SlotModule { + type SlotName = T[number]; + + const SlotContext = createContext | null>(null); + const OutletNameContext = createContext(null); + + const Slot = makeSlotConsumer(SlotContext) as SlotModule; + Slot.Provider = makeSlotProvider(SlotContext); + Slot.Outlet = makeSlotOutlet(SlotContext, OutletNameContext); + Slot.Fallback = makeSlotFallback(SlotContext, OutletNameContext); + + // Keep `names` reference to preserve the const-narrowed type T + void names; + + return Slot; +} + +export function withSlots< + TComponent extends React.ComponentType, + TSlot extends Slot, +>( + Component: TComponent, + slotModule: SlotModule +): TComponent & {Slot: SlotModule} { + const WithSlots = Component as TComponent & {Slot: SlotModule}; + WithSlots.Slot = slotModule; + return WithSlots; +} diff --git a/static/app/components/loading/useCycleText.tsx b/static/app/components/loading/useCycleText.tsx new file mode 100644 index 000000000000..da12245f087f --- /dev/null +++ b/static/app/components/loading/useCycleText.tsx @@ -0,0 +1,31 @@ +import {useEffect, useState} from 'react'; + +import {useTimeout} from 'sentry/utils/useTimeout'; + +interface Props { + delayMs: number; + messages: string[]; + disabled?: boolean; +} + +export function useCycleText({messages, delayMs, disabled = false}: Props) { + const [messageIndex, setMessageIndex] = useState(0); + + const {start, cancel} = useTimeout({ + timeMs: delayMs, + onTimeout: () => { + setMessageIndex(prev => Math.min(prev + 1, messages.length - 1)); + start(); + }, + }); + + useEffect(() => { + if (disabled) { + cancel(); + } else { + start(); + } + }, [start, cancel, disabled]); + + return messages[messageIndex]; +} diff --git a/static/app/components/repositories/scmIntegrationTree/scmIntegrationTreeRow.tsx b/static/app/components/repositories/scmIntegrationTree/scmIntegrationTreeRow.tsx index bfbc3fca0376..35c291c05d7e 100644 --- a/static/app/components/repositories/scmIntegrationTree/scmIntegrationTreeRow.tsx +++ b/static/app/components/repositories/scmIntegrationTree/scmIntegrationTreeRow.tsx @@ -1,4 +1,4 @@ -import {Fragment, useEffect, useState, type CSSProperties, type ReactNode} from 'react'; +import {Fragment, useState, type CSSProperties, type ReactNode} from 'react'; import styled from '@emotion/styled'; import {Badge, Tag} from '@sentry/scraps/badge'; @@ -10,6 +10,7 @@ import {Text} from '@sentry/scraps/text'; import {Tooltip} from '@sentry/scraps/tooltip'; import {hasEveryAccess} from 'sentry/components/acl/access'; +import {useCycleText} from 'sentry/components/loading/useCycleText'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {RepoProviderIcon} from 'sentry/components/repositories/repoProviderIcon'; import {ProviderConfigLink} from 'sentry/components/repositories/scmIntegrationTree/providerConfigLink'; @@ -24,7 +25,6 @@ import type { } from 'sentry/types/integrations'; import {isActiveSuperuser} from 'sentry/utils/isActiveSuperuser'; import {useOrganization} from 'sentry/utils/useOrganization'; -import {useTimeout} from 'sentry/utils/useTimeout'; import {AddIntegrationButton} from 'sentry/views/settings/organizationIntegrations/addIntegrationButton'; // --------------------------------------------------------------------------- @@ -458,30 +458,20 @@ function RemoveButton({ ); } +const messages = [ + t('Loading repos...'), + t('Loading a few repos...'), + t('Loading a lot more repos...'), + t('This is getting interesting...'), + t('Almost done...'), + t('Just kidding, still loading...'), +]; function LoadingReposMessage() { - const [messageIndex, setMessageIndex] = useState(0); - const {start} = useTimeout({ - timeMs: 5000, - onTimeout: () => { - setMessageIndex(prev => prev + 1); - start(); - }, - }); - useEffect(start, [start]); - - const messages = [ - t('Loading repos...'), - t('Loading a few repos...'), - t('Loading a lot more repos...'), - t('This is getting interesting...'), - t('Almost done...'), - t('Just kidding, still loading...'), - ]; - + const message = useCycleText({messages, delayMs: 5000}); return ( - {messages[Math.min(messageIndex, messages.length - 1)]} + {message} diff --git a/static/app/types/organization.tsx b/static/app/types/organization.tsx index 1db0d39c735c..2467c50b1452 100644 --- a/static/app/types/organization.tsx +++ b/static/app/types/organization.tsx @@ -1,3 +1,4 @@ +import type {AutofixStoppingPoint} from 'sentry/components/events/autofix/types'; import type {AggregationOutputType} from 'sentry/utils/discover/fields'; import type { DatasetSource, @@ -64,6 +65,7 @@ export interface Organization extends OrganizationSummary { dataScrubber: boolean; dataScrubberDefaults: boolean; debugFilesRole: string; + defaultAutomatedRunStoppingPoint: AutofixStoppingPoint; defaultCodeReviewTriggers: CodeReviewTrigger[]; defaultCodingAgent: string | null; defaultCodingAgentIntegrationId: string | number | null; diff --git a/static/app/utils/supergroup/useSuperGroups.spec.tsx b/static/app/utils/supergroup/useSuperGroups.spec.tsx new file mode 100644 index 000000000000..123f5cc90573 --- /dev/null +++ b/static/app/utils/supergroup/useSuperGroups.spec.tsx @@ -0,0 +1,78 @@ +import {OrganizationFixture} from 'sentry-fixture/organization'; + +import {renderHookWithProviders, waitFor} from 'sentry-test/reactTestingLibrary'; + +import {useSuperGroups} from 'sentry/utils/supergroup/useSuperGroups'; +import type {SupergroupDetail} from 'sentry/views/issueList/supergroups/types'; + +const organization = OrganizationFixture({features: ['top-issues-ui']}); +const API_URL = `/organizations/${organization.slug}/seer/supergroups/by-group/`; + +function makeSupergroup(overrides: Partial = {}): SupergroupDetail { + return { + id: 1, + title: 'Test Supergroup', + summary: 'A test supergroup', + error_type: 'TypeError', + code_area: 'frontend', + group_ids: [1, 2, 3], + created_at: '2024-01-01T00:00:00Z', + updated_at: '2024-01-01T00:00:00Z', + ...overrides, + }; +} + +describe('useSuperGroups', () => { + it('does not show loading state when archiving a group backfills a new one', async () => { + const supergroup = makeSupergroup({group_ids: [1, 2, 3]}); + const mockRequest = MockApiClient.addMockResponse({ + url: API_URL, + body: {data: [supergroup]}, + }); + + const {result, rerender} = renderHookWithProviders( + (props: {groupIds: string[]}) => useSuperGroups(props.groupIds), + { + organization, + initialProps: {groupIds: ['1', '2', '3']}, + } + ); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(mockRequest).toHaveBeenCalledTimes(1); + + // Group '3' is archived and removed from the list + rerender({groupIds: ['1', '2']}); + expect(result.current.isLoading).toBe(false); + expect(mockRequest).toHaveBeenCalledTimes(1); + + // A new group backfills into the list — should refetch in the background + rerender({groupIds: ['1', '2', '4']}); + expect(result.current.isLoading).toBe(false); + + await waitFor(() => expect(mockRequest).toHaveBeenCalledTimes(2)); + }); + + it('shows loading state when navigating to an entirely new page', async () => { + const supergroup = makeSupergroup({group_ids: [1, 2, 3]}); + const mockRequest = MockApiClient.addMockResponse({ + url: API_URL, + body: {data: [supergroup]}, + }); + + const {result, rerender} = renderHookWithProviders( + (props: {groupIds: string[]}) => useSuperGroups(props.groupIds), + { + organization, + initialProps: {groupIds: ['1', '2', '3']}, + } + ); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(mockRequest).toHaveBeenCalledTimes(1); + + // Navigate to a completely different page of results + rerender({groupIds: ['4', '5', '6']}); + expect(result.current.isLoading).toBe(true); + }); +}); diff --git a/static/app/utils/supergroup/useSuperGroups.tsx b/static/app/utils/supergroup/useSuperGroups.tsx index 8385fb02ac15..f80c2b444615 100644 --- a/static/app/utils/supergroup/useSuperGroups.tsx +++ b/static/app/utils/supergroup/useSuperGroups.tsx @@ -19,20 +19,21 @@ export function useSuperGroups(groupIds: string[]): { const organization = useOrganization(); const requestedGroupIdsRef = useRef(groupIds); const hasTopIssuesUI = organization.features.includes('top-issues-ui'); - const shouldReuseRequestedGroupIds = useMemo(() => { - const requestedGroupIds = requestedGroupIdsRef.current; - if (groupIds.length === 0 || requestedGroupIds.length < groupIds.length) { - return false; - } + const previousRequestedGroupIds = requestedGroupIdsRef.current; - const requestedGroupIdSet = new Set(requestedGroupIds); - return groupIds.every(groupId => requestedGroupIdSet.has(groupId)); + // Stabilize the query key: if the new groupIds are a subset of what we + // already requested (groups were removed), reuse the previous set to + // avoid a redundant refetch. + const requestedGroupIds = useMemo(() => { + const prev = requestedGroupIdsRef.current; + if (groupIds.length === 0 || prev.length < groupIds.length) { + return groupIds; + } + const prevSet = new Set(prev); + return groupIds.every(id => prevSet.has(id)) ? prev : groupIds; }, [groupIds]); - const requestedGroupIds = shouldReuseRequestedGroupIds - ? requestedGroupIdsRef.current - : groupIds; requestedGroupIdsRef.current = requestedGroupIds; const enabled = hasTopIssuesUI && requestedGroupIds.length > 0; @@ -50,6 +51,13 @@ export function useSuperGroups(groupIds: string[]): { { staleTime: 30_000, enabled, + placeholderData: previousData => { + if (!previousData) { + return undefined; + } + const prevSet = new Set(previousRequestedGroupIds); + return groupIds.some(id => prevSet.has(id)) ? previousData : undefined; + }, } ); @@ -57,7 +65,6 @@ export function useSuperGroups(groupIds: string[]): { if (!response?.data) { return {}; } - const result: SupergroupLookup = Object.fromEntries(groupIds.map(id => [id, null])); for (const sg of response.data) { for (const groupId of sg.group_ids) { diff --git a/static/app/views/dashboards/widgetBuilder/components/newWidgetBuilder.tsx b/static/app/views/dashboards/widgetBuilder/components/newWidgetBuilder.tsx index 3434106587d9..00e100897c8b 100644 --- a/static/app/views/dashboards/widgetBuilder/components/newWidgetBuilder.tsx +++ b/static/app/views/dashboards/widgetBuilder/components/newWidgetBuilder.tsx @@ -104,6 +104,30 @@ export function WidgetBuilderV2({ } }, []); + const [mainContentPosition, setMainContentPosition] = useState<{ + left: number; + top: number; + } | null>(null); + useEffect(() => { + const mainContentElement = document.querySelector('main'); + + const observer = new ResizeObserver(entries => { + for (const entry of entries) { + if (entry.target === mainContentElement) { + const rect = entry.target.getBoundingClientRect(); + setMainContentPosition({top: rect.top, left: rect.left}); + } + } + }); + if (mainContentElement) { + observer.observe(mainContentElement); + } + + return () => { + observer.disconnect(); + }; + }, []); + const dimensions = useDimensions({elementRef: navigationElementRef}); const handleDragEnd = ({over}: any) => { @@ -168,12 +192,12 @@ export function WidgetBuilderV2({ ? isMediumScreen ? { left: 0, - top: `${dimensions.height ?? 0}px`, + top: `${mainContentPosition?.top ?? 0}px`, willChange: 'top', } : { left: `${dimensions.width ?? 0}px`, - top: 0, + top: `${mainContentPosition?.top ?? 0}px`, willChange: 'left', } : undefined diff --git a/static/app/views/detectors/list/common/detectorListActions.tsx b/static/app/views/detectors/list/common/detectorListActions.tsx index 0a14dccd1cdd..a37e6be31e86 100644 --- a/static/app/views/detectors/list/common/detectorListActions.tsx +++ b/static/app/views/detectors/list/common/detectorListActions.tsx @@ -5,6 +5,7 @@ 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} from 'sentry/views/detectors/pathnames'; @@ -13,9 +14,10 @@ import {useCanCreateDetector} from 'sentry/views/detectors/utils/useCanCreateDet interface DetectorListActionsProps { children?: React.ReactNode; + detectorType?: DetectorType; } -export function DetectorListActions({children}: DetectorListActionsProps) { +export function DetectorListActions({children, detectorType}: DetectorListActionsProps) { const organization = useOrganization(); const {selection} = usePageFilters(); @@ -29,7 +31,7 @@ export function DetectorListActions({children}: DetectorListActionsProps) { } diff --git a/static/app/views/detectors/list/cron.tsx b/static/app/views/detectors/list/cron.tsx index d291cfe7f475..ac861c9fa7b3 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/metric.tsx b/static/app/views/detectors/list/metric.tsx index c0de7b94a409..063e6bfcf0d5 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 fe6914cf811e..daea09353a6e 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/uptime.tsx b/static/app/views/detectors/list/uptime.tsx index dbbacafdcd32..fa385dbfc422 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/navigation/topBar.tsx b/static/app/views/navigation/topBar.tsx index 49f2d82012a2..973b2d161818 100644 --- a/static/app/views/navigation/topBar.tsx +++ b/static/app/views/navigation/topBar.tsx @@ -3,6 +3,7 @@ import {useTheme} from '@emotion/react'; import {Button} from '@sentry/scraps/button'; import {Flex} from '@sentry/scraps/layout'; import {SizeProvider} from '@sentry/scraps/sizeContext'; +import {slot, withSlots} from '@sentry/scraps/slot'; import {FeedbackButton} from 'sentry/components/feedbackButton/feedbackButton'; import {IconSeer} from 'sentry/icons'; @@ -12,10 +13,14 @@ import {useHasPageFrameFeature} from 'sentry/views/navigation/useHasPageFrameFea import {useExplorerPanel} from 'sentry/views/seerExplorer/useExplorerPanel'; import {isSeerExplorerEnabled} from 'sentry/views/seerExplorer/utils'; -import {NAVIGATION_MOBILE_TOPBAR_HEIGHT_WITH_PAGE_FRAME} from './constants'; -import {PRIMARY_HEADER_HEIGHT} from './constants'; +import { + NAVIGATION_MOBILE_TOPBAR_HEIGHT_WITH_PAGE_FRAME, + PRIMARY_HEADER_HEIGHT, +} from './constants'; -export function TopBar() { +const Slot = slot(['title', 'actions', 'feedback'] as const); + +function TopBarContent() { const theme = useTheme(); const organization = useOrganization({allowNull: true}); const hasPageFrame = useHasPageFrameFeature(); @@ -39,29 +44,45 @@ export function TopBar() { position="sticky" borderBottom="primary" top={0} - // Keep the top bar in a cascade slightly below the sidebar panel so that when the sidebar panel - // is in the hover preview state, the top bar does not sit over it. style={{ zIndex: theme.zIndex.sidebarPanel - 1, }} > - {/* @TODO(JonasBadalic): Implement breadcrumbs here */} - - + + {props => } + + + + + {props => } + + {organization && isSeerExplorerEnabled(organization) ? ( ) : null} - - {null} - + + + {props => ( + + {/* If no component registers a feedback button, show the default one */} + + + {null} + + + + )} + ); } + +export const TopBar = withSlots(TopBarContent, Slot); diff --git a/static/app/views/organizationLayout/index.tsx b/static/app/views/organizationLayout/index.tsx index 61486f222bce..0aa207b431b7 100644 --- a/static/app/views/organizationLayout/index.tsx +++ b/static/app/views/organizationLayout/index.tsx @@ -93,11 +93,13 @@ function AppLayout({organization}: LayoutProps) { {organization && } - - - -