diff --git a/.agents/skills/generate-snapshot-tests/SKILL.md b/.agents/skills/generate-snapshot-tests/SKILL.md new file mode 100644 index 00000000000000..8a59563c5e4ebe --- /dev/null +++ b/.agents/skills/generate-snapshot-tests/SKILL.md @@ -0,0 +1,352 @@ +--- +name: generate-snapshot-tests +description: Generate snapshot test files for Sentry frontend React components. Use when asked to "generate snapshot tests", "add snapshot tests", "create visual snapshots", "write snapshot tests", "add visual regression tests", or "snapshot this component". Accepts an optional component path or name via $ARGUMENTS. +type: workflow-process +--- + +# Generate Snapshot Tests + +Generate a `*.snapshots.tsx` file colocated with a Sentry React component, following the established pattern used by core design system components. + +## Step 1: Locate the Component + +If `$ARGUMENTS` is provided, treat it as a path or component name. Otherwise ask the user which component to snapshot. + +Search strategies: + +``` +static/app/components/core//.tsx +static/app/components/core//index.tsx +static/app/components/.tsx +static/app/components//index.tsx +``` + +Use Glob or Grep to find the file if the exact path is unknown. + +Read the component source file to understand: + +- The component's name and its exported `Props` / `Props` type +- Union types and enum-like string literals on props (e.g., `variant`, `priority`, `size`) +- Boolean toggle props (e.g., `disabled`, `checked`, `busy`) +- Whether the component is interactive (needs `onChange={() => {}}` or similar no-op handlers) + +## Step 2: Determine the Import Path + +| Condition | Import style | +| ----------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Component lives under `static/app/components/core/` AND is published as `@sentry/scraps/` | `import {ComponentName, type ComponentNameProps} from '@sentry/scraps/';` | +| Component lives under `static/app/components/core/` but is NOT in `@sentry/scraps` | `// eslint-disable-next-line @sentry/scraps/no-core-import -- SSR snapshot needs direct import to avoid barrel re-exports with heavy deps`
`import {ComponentName, type ComponentNameProps} from 'sentry/components/core/';` | +| All other components | `import {ComponentName, type ComponentNameProps} from 'sentry/components/';` | + +To check if a component is in `@sentry/scraps`, look for an existing import using `@sentry/scraps/` in neighboring files, or check if other snapshot files in the same directory use `@sentry/scraps`. + +## Step 3: Identify Props to Snapshot + +Read the TypeScript props and classify them: + +| Prop type | Action | +| --------------------------------------------------------- | -------------------------------------------- | +| Union of string literals (`'sm' \| 'md' \| 'lg'`) | Snapshot each value with `it.snapshot.each` | +| Boolean toggle with visual impact (`disabled`, `checked`) | Snapshot `true` and `false` states | +| Boolean flag with no visual test value | Skip or add a single named snapshot | +| `children` / `className` / `style` / event handlers | Skip — not visually interesting on their own | + +Prioritize props that change the component's visual appearance substantially. For interactive components (inputs, toggles), always include disabled/checked states. + +## Step 4: Write the Snapshot File + +Name the output file `.snapshots.tsx`, colocated with the component source file. + +### Required imports (always include) + +```tsx +import {ThemeProvider} from '@emotion/react'; + +import {ComponentName, type ComponentNameProps} from '@sentry/scraps/'; // or appropriate path + +// eslint-disable-next-line no-restricted-imports -- SSR snapshot rendering needs direct theme access +import {darkTheme, lightTheme} from 'sentry/utils/theme/theme'; + +const themes = {light: lightTheme, dark: darkTheme}; +``` + +### Core structure + +Always wrap in light/dark theme loop: + +```tsx +describe('ComponentName', () => { + describe.each(['light', 'dark'] as const)('%s', themeName => { + // ... snapshot cases here + }); +}); +``` + +### `it.snapshot.each` — for union prop variants + +Use when iterating over multiple values of a single prop: + +```tsx +it.snapshot.each(['info', 'warning', 'success', 'danger'])( + '%s', + variant => ( + +
+ Label +
+
+ ), + variant => ({theme: themeName, variant: String(variant)}) +); +``` + +The third argument to `it.snapshot.each` is the metadata function — include all props that vary in the snapshot. This metadata is used for snapshot naming and diffing. + +### `it.snapshot` — for single named snapshots + +Use for one-off states (disabled, checked combinations, etc.): + +```tsx +it.snapshot('disabled-unchecked', () => ( + +
+ {}} /> +
+
+)); +``` + +Pass metadata as a third argument when it adds useful snapshot context: + +```tsx +it.snapshot( + 'bold', + () => ( + +
+ Bold text +
+
+ ), + {theme: themeName} +); +``` + +### Sizing the container + +Match container sizing to what makes the component readable: + +| Situation | Wrapper | +| ------------------------------ | ---------------------------------------- | +| Default | `
` | +| Width-sensitive (alerts, text) | `
` | +| Narrow (icons, small controls) | `
` | + +### Interactive components + +For components that require event handlers (inputs, checkboxes, radios, switches), pass no-op handlers to satisfy required props: + +```tsx + {}} /> + {}} /> +``` + +## Step 5: Ordering snapshots within the theme loop + +Order cases from most impactful to least: + +1. Primary variant/priority prop (the most visible visual differentiator) +2. Secondary variant props +3. Size variants +4. State combinations (disabled+unchecked, disabled+checked) +5. Boolean modifiers (bold, italic, etc.) +6. Edge cases and combined props + +## Examples + +### Simple variant component (Button-style) + +```tsx +import {ThemeProvider} from '@emotion/react'; + +import {Button, type ButtonProps} from '@sentry/scraps/button'; + +// eslint-disable-next-line no-restricted-imports -- SSR snapshot rendering needs direct theme access +import {darkTheme, lightTheme} from 'sentry/utils/theme/theme'; + +const themes = {light: lightTheme, dark: darkTheme}; + +describe('Button', () => { + describe.each(['light', 'dark'] as const)('%s', themeName => { + it.snapshot.each([ + 'default', + 'primary', + 'danger', + 'warning', + 'link', + 'transparent', + ])( + '%s', + priority => ( + +
+ +
+
+ ), + priority => ({theme: themeName, priority: String(priority)}) + ); + }); +}); +``` + +### Interactive component with state combinations (Switch-style) + +```tsx +import {ThemeProvider} from '@emotion/react'; + +import {Switch, type SwitchProps} from '@sentry/scraps/switch'; + +// eslint-disable-next-line no-restricted-imports -- SSR snapshot rendering needs direct theme access +import {darkTheme, lightTheme} from 'sentry/utils/theme/theme'; + +const themes = {light: lightTheme, dark: darkTheme}; + +describe('Switch', () => { + describe.each(['light', 'dark'] as const)('theme-%s', themeName => { + it.snapshot.each(['sm', 'lg'])('size-%s-unchecked', size => ( + +
+ {}} /> +
+
+ )); + + it.snapshot.each(['sm', 'lg'])('size-%s-checked', size => ( + +
+ {}} /> +
+
+ )); + + it.snapshot('disabled-unchecked', () => ( + +
+ {}} /> +
+
+ )); + + it.snapshot('disabled-checked', () => ( + +
+ {}} /> +
+
+ )); + }); +}); +``` + +### Component with multiple independent variant props (Alert-style) + +When a component has multiple meaningful boolean or variant props that combine independently, add separate `it.snapshot.each` blocks per combination: + +```tsx +describe('Alert', () => { + describe.each(['light', 'dark'] as const)('%s', themeName => { + // Primary variants + it.snapshot.each([ + 'info', + 'warning', + 'success', + 'danger', + 'muted', + ])( + '%s', + variant => ( + +
+ This is a {variant} alert +
+
+ ), + variant => ({theme: themeName, variant: String(variant)}) + ); + + // Modifier combination: same variants but with showIcon={false} + it.snapshot.each([ + 'info', + 'warning', + 'success', + 'danger', + 'muted', + ])( + '%s-no-icon', + variant => ( + +
+ + This is a {variant} alert without icon + +
+
+ ), + variant => ({theme: themeName, variant: String(variant), showIcon: 'false'}) + ); + }); +}); +``` + +## Anti-Patterns + +```tsx +// ❌ Don't import theme from the barrel re-export +import {theme} from 'sentry/utils/theme'; + +// ✅ Import directly and suppress the lint warning +// eslint-disable-next-line no-restricted-imports -- SSR snapshot rendering needs direct theme access +import {darkTheme, lightTheme} from 'sentry/utils/theme/theme'; +``` + +```tsx +// ❌ Don't omit the metadata argument — snapshot names become ambiguous +it.snapshot.each(['a', 'b'])('%s', variant => ( + +)); + +// ✅ Include metadata that reflects all varying props +it.snapshot.each(['a', 'b'])( + '%s', + variant => , + variant => ({theme: themeName, variant: String(variant)}) +); +``` + +```tsx +// ❌ Don't snapshot implementation-detail props like className or style +it.snapshot('custom-class', () => ); +``` + +```tsx +// ❌ Don't use @sentry/scraps barrel import for components not in the scraps package +import {Badge} from '@sentry/scraps/badge'; // if Badge isn't published there + +// ✅ Use the direct path with the no-core-import suppression comment +// eslint-disable-next-line @sentry/scraps/no-core-import -- SSR snapshot needs direct import to avoid barrel re-exports with heavy deps +import {Badge} from 'sentry/components/core/badge/badge'; +``` + +## Checklist + +Before finishing: + +- [ ] File is named `.snapshots.tsx` and colocated with the component +- [ ] Both `light` and `dark` themes are covered via `describe.each` +- [ ] All primary variant/priority props are snapshotted +- [ ] Interactive components include disabled and checked/unchecked states +- [ ] `no-restricted-imports` ESLint suppression comment is present on the theme import +- [ ] Metadata argument is provided to `it.snapshot.each` calls +- [ ] No-op handlers (`onChange={() => {}}`) provided for required event props +- [ ] Import path uses `@sentry/scraps/` if available, otherwise the direct `sentry/components/...` path with the `no-core-import` suppression diff --git a/.envrc b/.envrc index fd198708414da3..7f53a77dfe71b5 100644 --- a/.envrc +++ b/.envrc @@ -93,7 +93,7 @@ export PYTHONUNBUFFERED=1 unset PYTHONPATH # increase node's memory limit, required for our webpacking -export NODE_OPTIONS="--max-old-space-size=4096" +export NODE_OPTIONS="--max-old-space-size=5120" # Frontend hot module reloader using `react-refresh` # Enable this by default for development envs (CI/deploys do not use envrc) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 23e9f2fc6468f9..ad6b4a2fd9239f 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -42,6 +42,7 @@ /static/app/utils/ @getsentry/app-frontend /src/sentry/web/frontend/ @getsentry/app-frontend /.agents/skills/sentry-javascript-bugs/ @getsentry/app-frontend +/.agents/skills/generate-snapshot-tests/ @getsentry/app-frontend ## Legal /LICENSE.md @getsentry/owners-legal diff --git a/.github/codeowners-coverage-baseline.txt b/.github/codeowners-coverage-baseline.txt index c7b63d380a879d..ccc06ad9b0d2c5 100644 --- a/.github/codeowners-coverage-baseline.txt +++ b/.github/codeowners-coverage-baseline.txt @@ -184,7 +184,6 @@ src/sentry/runner/commands/cleanup.py src/sentry/runner/commands/config.py src/sentry/runner/commands/configoptions.py src/sentry/runner/commands/createflag.py -src/sentry/runner/commands/createorg.py src/sentry/runner/commands/createuser.py src/sentry/runner/commands/devserver.py src/sentry/runner/commands/devservices.py @@ -1209,8 +1208,6 @@ static/app/debug/notifications/previews/slackPreview.tsx static/app/debug/notifications/previews/teamsPreview.tsx static/app/debug/notifications/types.ts static/app/debug/notifications/views/index.tsx -static/app/endpoints/organizations/organizationsConfigIntegrationsQueryOptions.ts -static/app/endpoints/organizations/organizationsIntegrationsQueryOptions.ts static/app/index.tsx static/app/locale.spec.tsx static/app/main.tsx @@ -1473,7 +1470,6 @@ static/app/views/unsubscribe/issue.spec.tsx static/app/views/unsubscribe/issue.tsx static/app/views/unsubscribe/project.spec.tsx static/app/views/unsubscribe/project.tsx -static/gsAdmin/components/addBillingMetricUsage.tsx static/gsAdmin/components/addGiftBudgetAction.spec.tsx static/gsAdmin/components/addGiftBudgetAction.tsx static/gsAdmin/components/addGiftEventsAction.spec.tsx @@ -1887,13 +1883,6 @@ tests/sentry/audit_log/test_register.py tests/sentry/autofix/__init__.py tests/sentry/autofix/test_utils.py tests/sentry/autofix/test_webhooks.py -tests/sentry/billing/__init__.py -tests/sentry/billing/platform/__init__.py -tests/sentry/billing/platform/core/__init__.py -tests/sentry/billing/platform/core/test_service.py -tests/sentry/billing/platform/services/__init__.py -tests/sentry/billing/platform/services/usage/__init__.py -tests/sentry/billing/platform/services/usage/test_service.py tests/sentry/buffer/__init__.py tests/sentry/buffer/test_base.py tests/sentry/buffer/test_redis.py @@ -1925,7 +1914,6 @@ tests/sentry/core/endpoints/test_organization_member_invite_index.py tests/sentry/core/endpoints/test_organization_member_team_details.py tests/sentry/core/endpoints/test_organization_projects.py tests/sentry/core/endpoints/test_organization_projects_experiment.py -tests/sentry/core/endpoints/test_organization_region.py tests/sentry/core/endpoints/test_organization_request_project_creation.py tests/sentry/core/endpoints/test_organization_teams.py tests/sentry/core/endpoints/test_organization_user_details.py @@ -2295,7 +2283,6 @@ tests/sentry/runner/commands/test_cleanup.py tests/sentry/runner/commands/test_config.py tests/sentry/runner/commands/test_configoptions.py tests/sentry/runner/commands/test_createflag.py -tests/sentry/runner/commands/test_createorg.py tests/sentry/runner/commands/test_createuser.py tests/sentry/runner/commands/test_init.py tests/sentry/runner/commands/test_migrations.py @@ -2499,108 +2486,6 @@ tests/sentry/users/web/test_account_identity.py tests/sentry/users/web/test_accounts.py tests/sentry/users/web/test_accounts_form.py tests/sentry/users/web/test_user_avatar.py -tests/sentry/utils/__init__.py -tests/sentry/utils/email/__init__.py -tests/sentry/utils/email/test_address.py -tests/sentry/utils/email/test_backend.py -tests/sentry/utils/email/test_list_resolver.py -tests/sentry/utils/email/test_message_builder.py -tests/sentry/utils/email/test_send_mail.py -tests/sentry/utils/email/test_signer.py -tests/sentry/utils/kafka/__init__.py -tests/sentry/utils/kafka/test_rebalance_delay.py -tests/sentry/utils/kvstore/__init__.py -tests/sentry/utils/kvstore/test_bigtable.py -tests/sentry/utils/kvstore/test_common.py -tests/sentry/utils/kvstore/test_compat.py -tests/sentry/utils/kvstore/test_encoding.py -tests/sentry/utils/locking/__init__.py -tests/sentry/utils/locking/backends/__init__.py -tests/sentry/utils/locking/backends/test_migration.py -tests/sentry/utils/locking/backends/test_redis.py -tests/sentry/utils/locking/test_lock.py -tests/sentry/utils/mockdata/__init__.py -tests/sentry/utils/mockdata/test_core.py -tests/sentry/utils/sdk_crashes/conftest.py -tests/sentry/utils/sdk_crashes/test_build_sdk_crash_detection_configs.py -tests/sentry/utils/sdk_crashes/test_event_stripper.py -tests/sentry/utils/sdk_crashes/test_path_replacer.py -tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py -tests/sentry/utils/sdk_crashes/test_sdk_crash_detection_cocoa.py -tests/sentry/utils/sdk_crashes/test_sdk_crash_detection_dart.py -tests/sentry/utils/sdk_crashes/test_sdk_crash_detection_dotnet.py -tests/sentry/utils/sdk_crashes/test_sdk_crash_detection_java.py -tests/sentry/utils/sdk_crashes/test_sdk_crash_detection_native.py -tests/sentry/utils/sdk_crashes/test_sdk_crash_detection_react_native.py -tests/sentry/utils/sdk_crashes/test_sdk_crash_detector.py -tests/sentry/utils/security/test_encrypted_field_key_store.py -tests/sentry/utils/security/test_orgauthtoken_token.py -tests/sentry/utils/sentry_apps/test_webhook_timeout.py -tests/sentry/utils/test_arroyo.py -tests/sentry/utils/test_arroyo_producer.py -tests/sentry/utils/test_assets.py -tests/sentry/utils/test_audit.py -tests/sentry/utils/test_auth.py -tests/sentry/utils/test_avatar.py -tests/sentry/utils/test_circuit_breaker.py -tests/sentry/utils/test_circuit_breaker2.py -tests/sentry/utils/test_codecs.py -tests/sentry/utils/test_committers.py -tests/sentry/utils/test_concurrent.py -tests/sentry/utils/test_confluent_producer.py -tests/sentry/utils/test_cursors.py -tests/sentry/utils/test_datastructures.py -tests/sentry/utils/test_dates.py -tests/sentry/utils/test_eap.py -tests/sentry/utils/test_event.py -tests/sentry/utils/test_event_frames.py -tests/sentry/utils/test_event_tracker.py -tests/sentry/utils/test_eventuser.py -tests/sentry/utils/test_exceptions.py -tests/sentry/utils/test_function_cache.py -tests/sentry/utils/test_geo.py -tests/sentry/utils/test_github.py -tests/sentry/utils/test_glob.py -tests/sentry/utils/test_hashlib.py -tests/sentry/utils/test_http.py -tests/sentry/utils/test_integrationdocs.py -tests/sentry/utils/test_iterators.py -tests/sentry/utils/test_json.py -tests/sentry/utils/test_jwt.py -tests/sentry/utils/test_kafka_config.py -tests/sentry/utils/test_letter_avatar.py -tests/sentry/utils/test_linksign.py -tests/sentry/utils/test_math.py -tests/sentry/utils/test_meta.py -tests/sentry/utils/test_metrics.py -tests/sentry/utils/test_not_set.py -tests/sentry/utils/test_numbers.py -tests/sentry/utils/test_outcome_aggregator.py -tests/sentry/utils/test_outcomes.py -tests/sentry/utils/test_patch_set.py -tests/sentry/utils/test_projectflags.py -tests/sentry/utils/test_query.py -tests/sentry/utils/test_ratelimits.py -tests/sentry/utils/test_redis.py -tests/sentry/utils/test_registry.py -tests/sentry/utils/test_retries.py -tests/sentry/utils/test_rollout.py -tests/sentry/utils/test_rust.py -tests/sentry/utils/test_safe.py -tests/sentry/utils/test_samples.py -tests/sentry/utils/test_sdk.py -tests/sentry/utils/test_services.py -tests/sentry/utils/test_session_store.py -tests/sentry/utils/test_signing.py -tests/sentry/utils/test_snowflake.py -tests/sentry/utils/test_snuba.py -tests/sentry/utils/test_strings.py -tests/sentry/utils/test_tag_normalization.py -tests/sentry/utils/test_time_window.py -tests/sentry/utils/test_types.py -tests/sentry/utils/test_urls.py -tests/sentry/utils/test_validators.py -tests/sentry/utils/test_zip.py tests/sentry/web/__init__.py tests/sentry/web/forms/test_accounts.py tests/sentry/web/frontend/__init__.py diff --git a/.github/workflows/frontend-optional.yml b/.github/workflows/frontend-optional.yml index 0ec51df7b67d7c..ed37687635ee17 100644 --- a/.github/workflows/frontend-optional.yml +++ b/.github/workflows/frontend-optional.yml @@ -12,7 +12,7 @@ concurrency: # hack for https://github.com/actions/cache/issues/810#issuecomment-1222550359 env: SEGMENT_DOWNLOAD_TIMEOUT_MINS: 3 - NODE_OPTIONS: '--max-old-space-size=8192' + NODE_OPTIONS: '--max-old-space-size=5120' jobs: files-changed: diff --git a/.github/workflows/frontend.yml b/.github/workflows/frontend.yml index 5ed8b2bb60e241..7a9e806239175f 100644 --- a/.github/workflows/frontend.yml +++ b/.github/workflows/frontend.yml @@ -15,7 +15,7 @@ concurrency: # hack for https://github.com/actions/cache/issues/810#issuecomment-1222550359 env: SEGMENT_DOWNLOAD_TIMEOUT_MINS: 3 - NODE_OPTIONS: '--max-old-space-size=8192' + NODE_OPTIONS: '--max-old-space-size=5120' jobs: files-changed: @@ -68,6 +68,7 @@ jobs: needs: files-changed name: eslint runs-on: ubuntu-24.04 + timeout-minutes: 15 steps: - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 diff --git a/src/sentry/features/temporary.py b/src/sentry/features/temporary.py index 0bf28bcf3fc8de..16af60e86bff45 100644 --- a/src/sentry/features/temporary.py +++ b/src/sentry/features/temporary.py @@ -444,6 +444,9 @@ def register_temporary_features(manager: FeatureManager) -> None: # Use workflow engine exclusively for ProjectRuleGroupHistoryIndexEndpoint.get and ProjectRuleStatsIndexEndpoint.get results. # See src/sentry/workflow_engine/docs/legacy_backport.md for context. manager.add("organizations:workflow-engine-projectrulegroupstats-get", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) + # Use workflow engine exclusively for legacy issue alert rule.get results. + # See src/sentry/workflow_engine/docs/legacy_backport.md for context. + manager.add("organizations:workflow-engine-issue-alert-endpoints-get", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) # Enable metric detector limits by plan type manager.add("organizations:workflow-engine-metric-detector-limit", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) # Enable EventUniqueUserFrequencyConditionWithConditions special alert condition diff --git a/src/sentry/grouping/parameterization.py b/src/sentry/grouping/parameterization.py index c762da26c9e1c3..85043c06239069 100644 --- a/src/sentry/grouping/parameterization.py +++ b/src/sentry/grouping/parameterization.py @@ -2,6 +2,7 @@ import re from collections import OrderedDict, defaultdict from collections.abc import Sequence +from ipaddress import ip_address, ip_interface, ip_network from typing import Callable from sentry.utils import metrics @@ -44,6 +45,25 @@ def _get_pattern(self, raw_pattern: str) -> str: return rf"{prefix}(?P<{self.name}>{raw_pattern}){postfix}" +def is_valid_ip(maybe_ip_str: str) -> bool: + # Validate the string by attempting to pass it to the three built-in factory functions for + # creating different types of ip address objects. If any of them succeeds, it's a valid IP. If + # all three raise an error, it's not. + for fn, kwargs in ( + (ip_address, {}), + (ip_interface, {}), + (ip_network, {"strict": False}), # `strict: False` allows host bits + ): + try: + fn(maybe_ip_str, **kwargs) + except ValueError: + pass + else: + return True + + return False + + DEFAULT_PARAMETERIZATION_REGEXES = [ ParameterizationRegex( name="email", @@ -63,42 +83,6 @@ def _get_pattern(self, raw_pattern: str) -> str: \b """, ), - ParameterizationRegex( - name="ip", - raw_pattern=r""" - # This negative lookbehind ensures two things (depending on the pattern): - # - We don't match starting in the middle of a valid set of initial characters - # - We don't match things like `::` when they appear in expressions like `SomeClass::someMethod()` - (? str: """, ), ParameterizationRegex(name="duration", raw_pattern=r"""\b(\d+ms) | (\d+(\.\d+)?s)\b"""), + # The IP pattern has to come after the date pattern, because times like 12:31:12 are also valid + # IPv6 addresses + ParameterizationRegex( + name="ip", + # The rules for IP address (specifically IPv6 addresses) are sufficiently complicated that + # trying to write a regex to handle all cases would be (indeed, has been) quite hard and + # error-prone. Instead, we use our regex to find things which look like they *might* be + # valid IPs, and then let python's built-in `ipaddress` functions verify that we're right. + raw_pattern=r""" + # IPv4-like strings + (::[fF]{4}:)? # Optional prefix mapping the IPv4 address which follows to IPv6 format + ( + \b + (\d{1,3}\.){3} # Three sets of 1-3 digits, each followed by a literal dot + \d{1,3} # Final set of 1-3 digits + (/\d{1,2})? # Optional CIDR suffix + \b + ) + | + # IPv6-like strings + # + # Note: We can't use word boundaries here as we did with IPv4s because IPv6s contain + # non-word characters. Instead, we use a negative lookbehind and a negative lookahead, + # respectively, to specify the beginning and end of the pattern. These protect against + # two things: + # - Cases where the initial or end characters are all valid, but there are too many of + # them. (IOW, we don't want to match `2345:...` inside of an otherwise-invalid IP + # like `12345:...`, and the same applies to `...:1234` inside of `...:12345`.) + # - Cases where `::` (which is a valid IPv6 address) appears inside of expressions + # like `SomeClass::someMethod()`, especially when the characters bordering the `::` + # are valid hex, like `Space::explore()`. + # This doesn't fix edge cases like `Fee::add()`, where it's all hex and also fewer than + # 5 characters on either side, but those are presumably pretty rare. + (?" if is_valid_ip(orig_value) else orig_value, + ), ParameterizationRegex( name="swift_txn_id", raw_pattern=r""" diff --git a/src/sentry/integrations/github/integration.py b/src/sentry/integrations/github/integration.py index 86cdfaf189359f..67b6a9b3a193f2 100644 --- a/src/sentry/integrations/github/integration.py +++ b/src/sentry/integrations/github/integration.py @@ -1254,6 +1254,10 @@ def get_step_data( "count": info.get("count"), } for info in enriched + # TODO(epurkhiser): Remove this filter once the legacy Django + # views are removed and _build_installation_info_with_counts is + # merged directly into this API pipeline step. + if info["installation_id"] != "-1" ], } diff --git a/src/sentry/preprod/api/endpoints/preprod_artifact_approve.py b/src/sentry/preprod/api/endpoints/preprod_artifact_approve.py new file mode 100644 index 00000000000000..091a9fb6f848c1 --- /dev/null +++ b/src/sentry/preprod/api/endpoints/preprod_artifact_approve.py @@ -0,0 +1,99 @@ +from __future__ import annotations + +import logging +from datetime import datetime, timezone + +from rest_framework.request import Request +from rest_framework.response import Response + +from sentry.api.api_owners import ApiOwner +from sentry.api.api_publish_status import ApiPublishStatus +from sentry.api.base import cell_silo_endpoint +from sentry.api.bases.organization import ( + OrganizationEndpoint, + OrganizationReleasePermission, +) +from sentry.models.organization import Organization +from sentry.preprod.models import PreprodArtifact, PreprodComparisonApproval +from sentry.preprod.vcs.status_checks.size.tasks import create_preprod_status_check_task +from sentry.preprod.vcs.status_checks.snapshots.tasks import ( + create_preprod_snapshot_status_check_task, +) + +logger = logging.getLogger(__name__) + +FEATURE_TYPE_MAP = { + "snapshots": PreprodComparisonApproval.FeatureType.SNAPSHOTS, + "size": PreprodComparisonApproval.FeatureType.SIZE, +} + +STATUS_CHECK_TASK_MAP = { + PreprodComparisonApproval.FeatureType.SNAPSHOTS: create_preprod_snapshot_status_check_task, + PreprodComparisonApproval.FeatureType.SIZE: create_preprod_status_check_task, +} + + +@cell_silo_endpoint +class OrganizationPreprodArtifactApproveEndpoint(OrganizationEndpoint): + owner = ApiOwner.EMERGE_TOOLS + publish_status = { + "POST": ApiPublishStatus.EXPERIMENTAL, + } + permission_classes = (OrganizationReleasePermission,) + + def post(self, request: Request, organization: Organization, artifact_id: str) -> Response: + feature_type_str = request.data.get("feature_type") + if feature_type_str not in FEATURE_TYPE_MAP: + return Response( + {"detail": f"feature_type must be one of: {', '.join(FEATURE_TYPE_MAP.keys())}"}, + status=400, + ) + + feature_type = FEATURE_TYPE_MAP[feature_type_str] + + try: + artifact = PreprodArtifact.objects.get( + id=artifact_id, + project__organization_id=organization.id, + ) + except (PreprodArtifact.DoesNotExist, ValueError): + return Response({"detail": "Artifact not found"}, status=404) + + # TODO(hybrid-cloud): approved_by is a User FK (control silo). This cell silo + # endpoint stores the ID, and the snapshot GET resolves it via User.objects.filter(). + # Both will need to use an RPC service when hybrid cloud enforcement is enabled. + # exists()+create() instead of get_or_create — no unique constraint on this model + # (see snapshots/tasks.py for rationale) + already_approved = PreprodComparisonApproval.objects.filter( + preprod_artifact=artifact, + preprod_feature_type=feature_type, + approved_by_id=request.user.id, + approval_status=PreprodComparisonApproval.ApprovalStatus.APPROVED, + ).exists() + + if already_approved: + return Response({"detail": "Already approved"}, status=200) + + PreprodComparisonApproval.objects.create( + preprod_artifact=artifact, + preprod_feature_type=feature_type, + approved_by_id=request.user.id, + approval_status=PreprodComparisonApproval.ApprovalStatus.APPROVED, + approved_at=datetime.now(timezone.utc), + ) + + PreprodComparisonApproval.objects.filter( + preprod_artifact=artifact, + preprod_feature_type=feature_type, + approval_status=PreprodComparisonApproval.ApprovalStatus.NEEDS_APPROVAL, + ).delete() + + task = STATUS_CHECK_TASK_MAP[feature_type] + task.apply_async( + kwargs={ + "preprod_artifact_id": artifact.id, + "caller": "approval_endpoint", + } + ) + + return Response({"detail": "Approved"}, status=201) diff --git a/src/sentry/preprod/api/endpoints/preprod_artifact_snapshot.py b/src/sentry/preprod/api/endpoints/preprod_artifact_snapshot.py index acc4bc531a029e..921e9c1d65e036 100644 --- a/src/sentry/preprod/api/endpoints/preprod_artifact_snapshot.py +++ b/src/sentry/preprod/api/endpoints/preprod_artifact_snapshot.py @@ -31,13 +31,15 @@ BuildDetailsVcsInfo, ) from sentry.preprod.api.models.snapshots.project_preprod_snapshot_models import ( + SnapshotApprovalInfo, + SnapshotApprover, SnapshotComparisonRunInfo, SnapshotDetailsApiResponse, SnapshotImageResponse, ) from sentry.preprod.api.schemas import VCS_ERROR_MESSAGES, VCS_SCHEMA_PROPERTIES from sentry.preprod.helpers.deletion import delete_artifacts_and_eap_data -from sentry.preprod.models import PreprodArtifact +from sentry.preprod.models import PreprodArtifact, PreprodComparisonApproval from sentry.preprod.snapshots.comparison_categorizer import ( CategorizedComparison, categorize_comparison_images, @@ -59,6 +61,7 @@ ) from sentry.ratelimits.config import RateLimitConfig from sentry.types.ratelimit import RateLimit, RateLimitCategory +from sentry.users.models.user import User from sentry.utils import metrics logger = logging.getLogger(__name__) @@ -134,7 +137,7 @@ def delete(self, request: Request, organization: Organization, snapshot_id: str) except Exception: logger.exception( "preprod_snapshot.delete_failed", - extra={"artifact_id": int(snapshot_id)}, + extra={"artifact_id": artifact.id}, ) return Response( {"detail": "Internal error deleting snapshot."}, @@ -155,7 +158,7 @@ def delete(self, request: Request, organization: Organization, snapshot_id: str) logger.info( "preprod_snapshot.deleted", extra={ - "artifact_id": int(snapshot_id), + "artifact_id": artifact.id, "user_id": request.user.id if request.user else None, "files_deleted": result.files_deleted, "size_metrics_deleted": result.size_metrics_deleted, @@ -175,7 +178,7 @@ def get(self, request: Request, organization: Organization, snapshot_id: str) -> artifact = PreprodArtifact.objects.select_related("commit_comparison").get( id=snapshot_id, project__organization_id=organization.id ) - except PreprodArtifact.DoesNotExist: + except (PreprodArtifact.DoesNotExist, ValueError): return Response({"detail": "Snapshot not found"}, status=404) try: @@ -331,6 +334,77 @@ def get(self, request: Request, organization: Organization, snapshot_id: str) -> duration_ms=int(duration.total_seconds() * 1000), ) + approval_info: SnapshotApprovalInfo | None = None + all_approvals = list( + PreprodComparisonApproval.objects.filter( + preprod_artifact=artifact, + preprod_feature_type=PreprodComparisonApproval.FeatureType.SNAPSHOTS, + ) + ) + approved = [ + a + for a in all_approvals + if a.approval_status == PreprodComparisonApproval.ApprovalStatus.APPROVED + ] + + if approved: + sentry_user_ids = list({a.approved_by_id for a in approved if a.approved_by_id}) + users_by_id = {u.id: u for u in User.objects.filter(id__in=sentry_user_ids)} + + approver_list: list[SnapshotApprover] = [] + seen_approver_keys: set[str] = set() + for approval in approved: + if approval.approved_by_id: + key = f"sentry:{approval.approved_by_id}" + if key in seen_approver_keys: + continue + seen_approver_keys.add(key) + user = users_by_id.get(approval.approved_by_id) + if user: + approver_list.append( + SnapshotApprover( + id=str(user.id), + name=user.get_display_name(), + email=user.email, + username=user.username, + approved_at=approval.approved_at.isoformat() + if approval.approved_at + else None, + source="sentry", + ) + ) + elif approval.extras and "github" in approval.extras: + gh = approval.extras["github"] + gh_id = gh.get("id") + key = f"github:{gh_id or gh.get('login')}" + if key in seen_approver_keys: + continue + seen_approver_keys.add(key) + approver_list.append( + SnapshotApprover( + id=str(gh_id) if gh_id is not None else None, + name=gh.get("login"), + username=gh.get("login"), + avatar_url=f"https://avatars.githubusercontent.com/u/{gh_id}" + if gh_id is not None + else None, + approved_at=approval.approved_at.isoformat() + if approval.approved_at + else None, + source="github", + ) + ) + approval_info = SnapshotApprovalInfo( + status="approved", + approvers=approver_list, + ) + elif all_approvals: + # If records exist but none are APPROVED, they must be NEEDS_APPROVAL + approval_info = SnapshotApprovalInfo( + status="requires_approval", + approvers=[], + ) + return Response( SnapshotDetailsApiResponse( head_artifact_id=str(artifact.id), @@ -354,6 +428,7 @@ def get(self, request: Request, organization: Organization, snapshot_id: str) -> errored=categorized.errored, errored_count=len(categorized.errored), comparison_run_info=run_info, + approval_info=approval_info, ).dict() ) diff --git a/src/sentry/preprod/api/endpoints/urls.py b/src/sentry/preprod/api/endpoints/urls.py index dd66ba17f8ca30..259c1b9b494491 100644 --- a/src/sentry/preprod/api/endpoints/urls.py +++ b/src/sentry/preprod/api/endpoints/urls.py @@ -21,6 +21,7 @@ from .organization_preprod_retention import OrganizationPreprodRetentionEndpoint from .preprod_artifact_admin_batch_delete import PreprodArtifactAdminBatchDeleteEndpoint from .preprod_artifact_admin_info import PreprodArtifactAdminInfoEndpoint +from .preprod_artifact_approve import OrganizationPreprodArtifactApproveEndpoint from .preprod_artifact_rerun_analysis import ( PreprodArtifactAdminRerunAnalysisEndpoint, PreprodArtifactRerunAnalysisEndpoint, @@ -188,6 +189,12 @@ OrganizationPreprodPublicSizeAnalysisEndpoint.as_view(), name="sentry-api-0-organization-preprod-artifact-public-size-analysis", ), + # Approvals + re_path( + r"^(?P[^/]+)/preprodartifacts/(?P[^/]+)/approve/$", + OrganizationPreprodArtifactApproveEndpoint.as_view(), + name="sentry-api-0-organization-preprod-artifact-approve", + ), # Snapshots re_path( r"^(?P[^/]+)/preprodartifacts/snapshots/(?P[^/]+)/$", diff --git a/src/sentry/preprod/api/models/snapshots/project_preprod_snapshot_models.py b/src/sentry/preprod/api/models/snapshots/project_preprod_snapshot_models.py index 5094f60d41d703..a9c3c8efc98cea 100644 --- a/src/sentry/preprod/api/models/snapshots/project_preprod_snapshot_models.py +++ b/src/sentry/preprod/api/models/snapshots/project_preprod_snapshot_models.py @@ -1,6 +1,7 @@ from __future__ import annotations from enum import StrEnum +from typing import Literal from pydantic import BaseModel @@ -45,6 +46,21 @@ class SnapshotComparisonRunInfo(BaseModel): duration_ms: int | None = None +class SnapshotApprover(BaseModel): + id: str | None = None + name: str | None = None + email: str | None = None + username: str | None = None + avatar_url: str | None = None + approved_at: str | None = None + source: Literal["sentry", "github"] = "sentry" + + +class SnapshotApprovalInfo(BaseModel): + status: Literal["approved", "requires_approval"] + approvers: list[SnapshotApprover] = [] + + class SnapshotDetailsApiResponse(BaseModel): head_artifact_id: str base_artifact_id: str | None = None @@ -78,5 +94,7 @@ class SnapshotDetailsApiResponse(BaseModel): comparison_run_info: SnapshotComparisonRunInfo | None = None + approval_info: SnapshotApprovalInfo | None = None + # TODO: POST request in the future when we migrate away from current schemas diff --git a/src/sentry/seer/explorer/tools.py b/src/sentry/seer/explorer/tools.py index 7d7d7cc1fcf86e..7764c7cb40b1c3 100644 --- a/src/sentry/seer/explorer/tools.py +++ b/src/sentry/seer/explorer/tools.py @@ -34,6 +34,7 @@ from sentry.search.eap.constants import BOOLEAN, DOUBLE, INT, STRING from sentry.search.eap.resolver import SearchResolver from sentry.search.eap.types import SearchResolverConfig +from sentry.search.events.constants import ISSUE_ID_ALIAS from sentry.search.events.types import SAMPLING_MODES, SnubaParams from sentry.seer.autofix.autofix import get_all_tags_overview from sentry.seer.constants import SEER_SUPPORTED_SCM_PROVIDERS @@ -776,7 +777,7 @@ def _get_issue_event_timeseries( dataset=dataset, y_axes=["count()"], group_by=[], - query=f"issue:{group.qualified_short_id}", + query=f"{ISSUE_ID_ALIAS}:{group.id}", start=start.isoformat(), end=end.isoformat(), interval=interval, diff --git a/src/sentry/workflow_engine/endpoints/validators/api_docs_help_text.py b/src/sentry/workflow_engine/endpoints/validators/api_docs_help_text.py index 05838fc6fab2f7..984376ed4ffb06 100644 --- a/src/sentry/workflow_engine/endpoints/validators/api_docs_help_text.py +++ b/src/sentry/workflow_engine/endpoints/validators/api_docs_help_text.py @@ -601,7 +601,26 @@ "targetDisplay":"" }, "integrationId":"2345", - "data":{...}, + "data":{ + "additional_fields": { + "assignee": "", + "integration": "2345", + "labels": [], + "repo": "example-repo", + }, + "dynamic_form_fields": [ + { + "choices": [["YourOrg/example-repo", "example-repo"]], + "default": "YourOrg/example-repo", + "label": "GitHub Repository", + "name": "repo", + "required": true + "type": "select", + "updatesForm": true, + "url": "/extensions/github/search/example-repo/1234567/", + }, + ], + }, "status":"active" } ``` diff --git a/src/sentry/workflow_engine/endpoints/validators/base/data_source.py b/src/sentry/workflow_engine/endpoints/validators/base/data_source.py index 02c28d42896425..b88ff1b3f30563 100644 --- a/src/sentry/workflow_engine/endpoints/validators/base/data_source.py +++ b/src/sentry/workflow_engine/endpoints/validators/base/data_source.py @@ -11,6 +11,10 @@ T = TypeVar("T", bound=Model) +# TODO - make this a typed dict +DataSourceInput = dict[str, Any] + + class DataSourceCreator(Generic[T]): def __init__(self, create_fn: Callable[[], T]): self._create_fn = create_fn diff --git a/src/sentry/workflow_engine/endpoints/validators/base/detector.py b/src/sentry/workflow_engine/endpoints/validators/base/detector.py index 935ca59f61b271..7b55ed73cf862a 100644 --- a/src/sentry/workflow_engine/endpoints/validators/base/detector.py +++ b/src/sentry/workflow_engine/endpoints/validators/base/detector.py @@ -1,6 +1,6 @@ import builtins from dataclasses import dataclass -from typing import Any +from typing import Any, NotRequired, TypedDict from django.db import router, transaction from jsonschema import ValidationError as JSONSchemaValidationError @@ -24,6 +24,10 @@ BaseDataConditionGroupValidator, BaseDataConditionValidator, ) +from sentry.workflow_engine.endpoints.validators.base.data_condition_group import ( + DataConditionGroupInput, +) +from sentry.workflow_engine.endpoints.validators.base.data_source import DataSourceInput from sentry.workflow_engine.endpoints.validators.utils import ( connect_detectors_to_workflows, get_unknown_detector_type_error, @@ -48,6 +52,17 @@ class DetectorQuota: count: int +class DetectorInput(TypedDict): + name: str + type: str + data_sources: NotRequired[list[DataSourceInput]] + config: NotRequired[dict[str, Any]] + condition_group: NotRequired[DataConditionGroupInput] + owner: NotRequired[str | int | None] + description: NotRequired[str] + enabled: NotRequired[bool] + + class BaseDetectorTypeValidator(CamelSnakeSerializer[Any]): enforce_single_datasource = False """ diff --git a/static/app/components/core/input/inputGroup.snapshots.tsx b/static/app/components/core/input/inputGroup.snapshots.tsx new file mode 100644 index 00000000000000..4115e463bdc891 --- /dev/null +++ b/static/app/components/core/input/inputGroup.snapshots.tsx @@ -0,0 +1,64 @@ +import {ThemeProvider} from '@emotion/react'; + +import {InputGroup} from '@sentry/scraps/input'; +import type {InputProps} from '@sentry/scraps/input'; + +import {IconSearch} from 'sentry/icons'; +// eslint-disable-next-line no-restricted-imports -- SSR snapshot rendering needs direct theme access +import {darkTheme, lightTheme} from 'sentry/utils/theme/theme'; + +const themes = {light: lightTheme, dark: darkTheme}; + +describe('InputGroup', () => { + describe.each(['light', 'dark'] as const)('%s', themeName => { + it.snapshot.each(['md', 'sm', 'xs'])( + 'size-%s', + size => ( + +
+ + + +
+
+ ), + size => ({theme: themeName, size: String(size)}) + ); + + it.snapshot('disabled', () => ( + +
+ + + +
+
+ )); + + it.snapshot('with-leading-items', () => ( + +
+ + + + + + +
+
+ )); + + it.snapshot('with-leading-items-disabled', () => ( + +
+ + + + + + +
+
+ )); + }); +}); diff --git a/static/app/components/stackTrace/frame/frameContent.tsx b/static/app/components/stackTrace/frame/frameContent.tsx index 57d63a200477cf..f1419b13a015f3 100644 --- a/static/app/components/stackTrace/frame/frameContent.tsx +++ b/static/app/components/stackTrace/frame/frameContent.tsx @@ -2,7 +2,7 @@ import {Activity, useRef} from 'react'; import {css} from '@emotion/react'; import styled from '@emotion/styled'; -import {Container, Grid} from '@sentry/scraps/layout'; +import {Container} from '@sentry/scraps/layout'; import {Text} from '@sentry/scraps/text'; import {Tooltip} from '@sentry/scraps/tooltip'; @@ -47,11 +47,6 @@ export function FrameContent({sourceLineCoverage = []}: FrameContentProps) { } const contextLines = isExpanded ? (frame.context ?? []) : []; - const maxLineNumber = contextLines.reduce( - (max, [lineNo]) => Math.max(max, lineNo ?? 0), - 0 - ); - const lineNumberDigits = String(maxLineNumber).length; const fileExtension = isExpanded ? (getFileExtension(frame.filename ?? '') ?? '') : ''; const prismLines = usePrismTokensSourceContext({ contextLines, @@ -89,7 +84,6 @@ export function FrameContent({sourceLineCoverage = []}: FrameContentProps) { ` - grid-template-columns: - calc(${p => Math.max(p.lineNumberDigits, 3) + 1}ch) - 1fr; +const FrameSourceRow = styled('div')<{isActive: boolean}>` + display: grid; + grid-column: 1 / -1; + grid-template-columns: subgrid; align-items: start; - column-gap: ${p => p.theme.space.sm}; min-width: 0; background: ${p => (p.isActive ? p.theme.tokens.background.secondary : 'transparent')}; `; @@ -186,8 +180,7 @@ const FrameSourceLineNumber = styled('div')<{ line-height: 1.8; text-align: right; user-select: none; - padding-left: ${p => p.theme.space.xs}; - padding-right: ${p => p.theme.space.xs}; + padding-inline: 1ch; border-right: 3px solid transparent; ${p => @@ -242,8 +235,9 @@ const FrameSourceCode = styled('code')` line-height: 1.8; display: block; min-width: 0; + padding-inline: 1ch; background: transparent; - padding: 0; + padding-block: 0; border-radius: 0; && { diff --git a/static/app/types/integrations.tsx b/static/app/types/integrations.tsx index 643cfe07f05265..ab33c5c35a937c 100644 --- a/static/app/types/integrations.tsx +++ b/static/app/types/integrations.tsx @@ -378,6 +378,7 @@ type IntegrationAspects = { configure_integration?: { title: string; }; + directEnable?: boolean; disable_dialog?: IntegrationDialog; externalInstall?: { buttonText: string; diff --git a/static/app/utils/api/knownSentryApiUrls.generated.ts b/static/app/utils/api/knownSentryApiUrls.generated.ts index 7368609bbf53d5..8dad1d57566d4c 100644 --- a/static/app/utils/api/knownSentryApiUrls.generated.ts +++ b/static/app/utils/api/knownSentryApiUrls.generated.ts @@ -475,6 +475,7 @@ export type KnownSentryApiUrls = | '/organizations/$organizationIdOrSlug/preprod-artifact/rerun-status-checks/$headArtifactId/' | '/organizations/$organizationIdOrSlug/preprod/quota/' | '/organizations/$organizationIdOrSlug/preprod/retention/' + | '/organizations/$organizationIdOrSlug/preprodartifacts/$artifactId/approve/' | '/organizations/$organizationIdOrSlug/preprodartifacts/$artifactId/install-details/' | '/organizations/$organizationIdOrSlug/preprodartifacts/$artifactId/size-analysis/' | '/organizations/$organizationIdOrSlug/preprodartifacts/$headArtifactId/build-details/' diff --git a/static/app/views/issueDetails/streamline/sidebar/autofixSection.spec.tsx b/static/app/views/issueDetails/streamline/sidebar/autofixSection.spec.tsx index 203d3ba2cfe8ba..7e325d7ac655a9 100644 --- a/static/app/views/issueDetails/streamline/sidebar/autofixSection.spec.tsx +++ b/static/app/views/issueDetails/streamline/sidebar/autofixSection.spec.tsx @@ -162,6 +162,10 @@ describe('AutofixSection', () => { expect(await screen.findByText('Root Cause')).toBeInTheDocument(); expect(screen.getByText('Null pointer in user handler')).toBeInTheDocument(); expect(screen.getByRole('button', {name: 'Open Seer'})).toBeInTheDocument(); + expect(screen.getByRole('button', {name: 'Open Seer'})).toHaveAttribute( + 'href', + expect.stringContaining('seerDrawer=true') + ); }); it('renders solution artifact', async () => { @@ -596,5 +600,9 @@ describe('AutofixSection', () => { expect(screen.getByText('Outline a plan')).toBeInTheDocument(); expect(screen.getByText('Create a code fix')).toBeInTheDocument(); expect(screen.getByRole('button', {name: 'Start Analysis'})).toBeInTheDocument(); + expect(screen.getByRole('button', {name: 'Start Analysis'})).toHaveAttribute( + 'href', + expect.stringContaining('seerDrawer=true') + ); }); }); diff --git a/static/app/views/issueDetails/streamline/sidebar/autofixSection.tsx b/static/app/views/issueDetails/streamline/sidebar/autofixSection.tsx index 24d449871693e4..2b53486b83a272 100644 --- a/static/app/views/issueDetails/streamline/sidebar/autofixSection.tsx +++ b/static/app/views/issueDetails/streamline/sidebar/autofixSection.tsx @@ -3,7 +3,7 @@ import styled from '@emotion/styled'; import seerConfigConnectImg from 'sentry-images/spot/seer-config-connect-2.svg'; -import {Button, LinkButton} from '@sentry/scraps/button'; +import {LinkButton} from '@sentry/scraps/button'; import {Image} from '@sentry/scraps/image'; import {Container, Flex} from '@sentry/scraps/layout'; import {Text} from '@sentry/scraps/text'; @@ -41,6 +41,7 @@ import type {Group} from 'sentry/types/group'; import type {Project} from 'sentry/types/project'; import {getConfigForIssueType} from 'sentry/utils/issueTypeConfig'; import {useRouteAnalyticsParams} from 'sentry/utils/routeAnalytics/useRouteAnalyticsParams'; +import {useLocation} from 'sentry/utils/useLocation'; import {useOrganization} from 'sentry/utils/useOrganization'; import {useSeerOnboardingCheck} from 'sentry/utils/useSeerOnboardingCheck'; import {SectionKey} from 'sentry/views/issueDetails/streamline/context'; @@ -247,6 +248,8 @@ function AutofixEmptyState({ project, referrer, }: AutofixEmptyStateProps) { + const seerDrawerLink = useSeerDrawerLink(); + const {openSeerDrawer} = useOpenSeerDrawer({ group, project, @@ -288,18 +291,19 @@ function AutofixEmptyState({ - + ); } @@ -319,6 +323,8 @@ function AutofixPreviews({ sections, referrer, }: AutofixPreviewsProps) { + const seerDrawerLink = useSeerDrawerLink(); + const hasRootCause = sections.findLast(isRootCauseSection)?.artifacts?.some(isRootCauseArtifact) ?? false; @@ -377,11 +383,12 @@ function AutofixPreviews({ // TODO: maybe send a log? return null; })} - + ); } +function useSeerDrawerLink() { + const location = useLocation(); + return { + pathname: location.pathname, + query: { + ...location.query, + seerDrawer: true, + }, + }; +} + const ImageContainer = styled(Flex)<{ aspectRatio?: CSSProperties['aspectRatio']; }>` diff --git a/static/app/views/preprod/snapshots/header/snapshotDevTools.tsx b/static/app/views/preprod/snapshots/header/snapshotDevTools.tsx deleted file mode 100644 index d7c296fddb46b3..00000000000000 --- a/static/app/views/preprod/snapshots/header/snapshotDevTools.tsx +++ /dev/null @@ -1,257 +0,0 @@ -import {useCallback, useEffect, useMemo, useRef, useState} from 'react'; -import {keyframes} from '@emotion/react'; -import styled from '@emotion/styled'; - -import {Button} from '@sentry/scraps/button'; -import {Flex} from '@sentry/scraps/layout'; -import {Text} from '@sentry/scraps/text'; - -import {Client} from 'sentry/api'; -import {IconAdd, IconSubtract} from 'sentry/icons'; -import {t} from 'sentry/locale'; -import {formatDuration} from 'sentry/utils/duration/formatDuration'; -import { - ComparisonState, - type SnapshotComparisonRunInfo, -} from 'sentry/views/preprod/types/snapshotTypes'; - -interface SnapshotDevToolsProps { - hasBaseArtifact: boolean; - isSoloView: boolean; - onToggleView: () => void; - organizationSlug: string; - refetch: () => void; - snapshotId: string; - comparisonRunInfo?: SnapshotComparisonRunInfo | null; -} - -export function SnapshotDevTools({ - organizationSlug, - snapshotId, - comparisonRunInfo, - hasBaseArtifact, - refetch, - isSoloView, - onToggleView, -}: SnapshotDevToolsProps) { - const comparisonState = comparisonRunInfo?.state; - const comparisonCompletedAt = comparisonRunInfo?.completed_at; - const comparisonDurationMs = comparisonRunInfo?.duration_ms; - const [devToolsCollapsed, setDevToolsCollapsed] = useState( - () => localStorage.getItem('snapshot-dev-tools-collapsed') === 'true' - ); - const [recompareLoading, setRecompareLoading] = useState(false); - const [recompareError, setRecompareError] = useState(null); - const clientRef = useRef(new Client()); - - const polling = useMemo( - () => - comparisonState === ComparisonState.PENDING || - comparisonState === ComparisonState.PROCESSING, - [comparisonState] - ); - - useEffect(() => { - if (!polling) { - return undefined; - } - const interval = setInterval(() => refetch(), 1000); - return () => clearInterval(interval); - }, [polling, refetch]); - - const setCollapsed = (collapsed: boolean) => { - setDevToolsCollapsed(collapsed); - localStorage.setItem('snapshot-dev-tools-collapsed', String(collapsed)); - }; - - const handleRecompare = useCallback(() => { - setRecompareLoading(true); - setRecompareError(null); - clientRef.current.request( - `/organizations/${organizationSlug}/preprodartifacts/snapshots/${snapshotId}/recompare/`, - { - method: 'POST', - success: () => { - setRecompareLoading(false); - refetch(); - }, - error: (err: any) => { - setRecompareLoading(false); - setRecompareError(err?.responseJSON?.detail ?? 'Failed to recompare'); - }, - } - ); - }, [organizationSlug, snapshotId, refetch]); - - let stateLabel: string; - if (comparisonState === ComparisonState.PROCESSING) { - stateLabel = t('Processing...'); - } else if (comparisonState === ComparisonState.PENDING) { - stateLabel = t('Queued...'); - } else if (comparisonState === ComparisonState.FAILED) { - stateLabel = t('Failed'); - } else if (comparisonCompletedAt) { - stateLabel = t('Done'); - } else { - stateLabel = t('No comparison'); - } - - return ( - - {devToolsCollapsed ? ( - - - {t('temp dev tools')} - - - )} - {hasBaseArtifact && ( - - {'|'} - - )} - {hasBaseArtifact && ( - - )} - - )} - {!devToolsCollapsed && recompareError && ( - - {recompareError} - - )} - - ); -} - -const pulse = keyframes` - 0%, 100% { opacity: 1; } - 50% { opacity: 0.3; } -`; - -const DevToolsBox = styled('div')` - position: relative; - display: flex; - flex-direction: column; - gap: ${p => p.theme.space.xs}; - padding: ${p => p.theme.space.sm} ${p => p.theme.space.md}; - border: 1px dashed ${p => p.theme.tokens.border.primary}; - border-radius: ${p => p.theme.radius.md}; - - &:not([data-collapsed='true']) { - padding-right: 48px; - } -`; - -const CollapseButton = styled(Button)` - position: absolute; - top: ${p => p.theme.space.xs}; - right: ${p => p.theme.space.xs}; -`; - -const StatusPill = styled('div')` - display: flex; - align-items: center; - gap: ${p => p.theme.space.xs}; - padding: 2px ${p => p.theme.space.sm}; - border: 1px solid ${p => p.theme.tokens.border.accent}; - border-radius: 12px; -`; - -const PulsingDot = styled('div')<{active: boolean}>` - width: 8px; - height: 8px; - border-radius: 50%; - background: ${p => (p.active ? p.theme.colors.yellow300 : p.theme.colors.gray200)}; - animation: ${p => (p.active ? pulse : 'none')} 1.2s ease-in-out infinite; -`; diff --git a/static/app/views/preprod/snapshots/header/snapshotHeaderActions.tsx b/static/app/views/preprod/snapshots/header/snapshotHeaderActions.tsx new file mode 100644 index 00000000000000..3ec9bb755f69ee --- /dev/null +++ b/static/app/views/preprod/snapshots/header/snapshotHeaderActions.tsx @@ -0,0 +1,217 @@ +import {useCallback, useEffect, useRef, useState} from 'react'; + +import {AvatarList} from '@sentry/scraps/avatar'; +import {Tag} from '@sentry/scraps/badge'; +import {Button} from '@sentry/scraps/button'; +import {Flex} from '@sentry/scraps/layout'; +import {Text} from '@sentry/scraps/text'; + +import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator'; +import {Client} from 'sentry/api'; +import {ConfirmDelete} from 'sentry/components/confirmDelete'; +import {DropdownMenu} from 'sentry/components/dropdownMenu'; +import type {MenuItemProps} from 'sentry/components/dropdownMenu'; +import { + IconCheckmark, + IconDelete, + IconEllipsis, + IconRefresh, + IconThumb, + IconTimer, +} from 'sentry/icons'; +import {t} from 'sentry/locale'; +import type {AvatarUser} from 'sentry/types/user'; +import {useQueryClient} from 'sentry/utils/queryClient'; +import {useIsSentryEmployee} from 'sentry/utils/useIsSentryEmployee'; +import {useNavigate} from 'sentry/utils/useNavigate'; +import type {SnapshotDetailsApiResponse} from 'sentry/views/preprod/types/snapshotTypes'; +import {handleStaffPermissionError} from 'sentry/views/preprod/utils/staffPermissionError'; + +interface SnapshotHeaderActionsProps { + apiUrl: string; + data: SnapshotDetailsApiResponse; + organizationSlug: string; +} + +export function SnapshotHeaderActions({ + data, + organizationSlug, + apiUrl, +}: SnapshotHeaderActionsProps) { + const queryClient = useQueryClient(); + const clientRef = useRef(new Client()); + useEffect(() => () => clientRef.current.clear(), []); + const navigate = useNavigate(); + const isSentryEmployee = useIsSentryEmployee(); + const [isApproving, setIsApproving] = useState(false); + const [isDeleting, setIsDeleting] = useState(false); + + const isApproved = data.approval_info?.status === 'approved'; + const approvers: AvatarUser[] = (data.approval_info?.approvers ?? []).map((a, i) => ({ + id: a.id ?? `approver-${i}`, + name: a.name ?? '', + email: a.email ?? '', + username: a.username ?? '', + ip_address: '', + avatar: a.avatar_url + ? { + avatarType: 'upload' as const, + avatarUuid: '', + avatarUrl: a.avatar_url, + } + : undefined, + })); + + const handleApprove = useCallback(() => { + setIsApproving(true); + clientRef.current.request( + `/organizations/${organizationSlug}/preprodartifacts/${data.head_artifact_id}/approve/`, + { + method: 'POST', + data: {feature_type: 'snapshots'}, + success: () => { + addSuccessMessage(t('Snapshot approved')); + queryClient.invalidateQueries({queryKey: [apiUrl]}); + setIsApproving(false); + }, + error: (resp: any) => { + setIsApproving(false); + if (resp?.status === 403) { + handleStaffPermissionError(resp?.responseJSON?.detail); + } else { + addErrorMessage(t('Failed to approve snapshot')); + } + }, + } + ); + }, [organizationSlug, data.head_artifact_id, queryClient, apiUrl]); + + const handleRerunComparison = useCallback(() => { + clientRef.current.request( + `/organizations/${organizationSlug}/preprod-artifact/rerun-status-checks/${data.head_artifact_id}/`, + { + method: 'POST', + data: {check_types: ['snapshots']}, + success: () => { + addSuccessMessage(t('Re-run comparison initiated')); + queryClient.invalidateQueries({queryKey: [apiUrl]}); + }, + error: (resp: any) => { + if (resp?.status === 403) { + handleStaffPermissionError(resp?.responseJSON?.detail); + } else { + addErrorMessage(t('Failed to re-run comparison')); + } + }, + } + ); + }, [organizationSlug, data.head_artifact_id, queryClient, apiUrl]); + + const handleDelete = useCallback(() => { + setIsDeleting(true); + clientRef.current.request(apiUrl, { + method: 'DELETE', + success: () => { + addSuccessMessage(t('Snapshot deleted')); + // TODO(preprod): Redirect to snapshot builds list once that UI is added + navigate('/'); + }, + error: (resp: any) => { + setIsDeleting(false); + if (resp?.status === 403) { + handleStaffPermissionError(resp?.responseJSON?.detail); + } else { + addErrorMessage(t('Failed to delete snapshot')); + } + }, + }); + }, [apiUrl, navigate]); + + return ( + + {data.approval_info && + (isApproved ? ( + + }> + {t('Approved')} + + {approvers.length > 0 && ( + + )} + + ) : ( + + }> + {t('Requires approval')} + + + + ))} + + + {({open: openDeleteModal}) => { + const menuItems: MenuItemProps[] = [ + { + key: 'delete', + label: ( + + + {t('Delete Snapshots')} + + ), + onAction: openDeleteModal, + textValue: t('Delete Snapshots'), + }, + ]; + + if (isSentryEmployee) { + menuItems.push({ + key: 'admin-section', + label: t('Admin (Sentry Employees only)'), + children: [ + { + key: 'rerun-comparison', + label: ( + + + {t('Re-run comparison')} + + ), + onAction: handleRerunComparison, + textValue: t('Re-run comparison'), + }, + ], + }); + } + + return ( + , + 'aria-label': t('More actions'), + disabled: isDeleting, + }} + /> + ); + }} + + + ); +} diff --git a/static/app/views/preprod/snapshots/snapshots.tsx b/static/app/views/preprod/snapshots/snapshots.tsx index 77edd78b1a79f0..0328df777a603a 100644 --- a/static/app/views/preprod/snapshots/snapshots.tsx +++ b/static/app/views/preprod/snapshots/snapshots.tsx @@ -3,7 +3,6 @@ import {useTheme} from '@emotion/react'; import styled from '@emotion/styled'; import {Flex, Stack} from '@sentry/scraps/layout'; -import {Text} from '@sentry/scraps/text'; import * as Layout from 'sentry/components/layouts/thirds'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; @@ -17,6 +16,7 @@ import {useNavigate} from 'sentry/utils/useNavigate'; import {useOrganization} from 'sentry/utils/useOrganization'; import {useParams} from 'sentry/utils/useParams'; import {useResizableDrawer} from 'sentry/utils/useResizableDrawer'; +import {BuildError} from 'sentry/views/preprod/components/buildError'; import {BuildProcessing} from 'sentry/views/preprod/components/buildProcessing'; import {ComparisonState, getImageGroup} from 'sentry/views/preprod/types/snapshotTypes'; import type { @@ -27,7 +27,7 @@ import type { } from 'sentry/views/preprod/types/snapshotTypes'; import {computeSidebarBadges} from 'sentry/views/preprod/utils/sidebarUtils'; -import {SnapshotDevTools} from './header/snapshotDevTools'; +import {SnapshotHeaderActions} from './header/snapshotHeaderActions'; import {SnapshotHeaderContent} from './header/snapshotHeaderContent'; import type {DiffMode} from './main/imageDisplay/diffImageDisplay'; import {SnapshotMainContent} from './main/snapshotMainContent'; @@ -44,21 +44,23 @@ export default function SnapshotsPage() { snapshotId: string; }>(); - const {data, isPending, isError, refetch} = useApiQuery( - [ - getApiUrl( - '/organizations/$organizationIdOrSlug/preprodartifacts/snapshots/$snapshotId/', - { - path: { - organizationIdOrSlug: organization.slug, - snapshotId, - }, - } - ), - ], + const snapshotApiUrl = getApiUrl( + '/organizations/$organizationIdOrSlug/preprodartifacts/snapshots/$snapshotId/', + { + path: { + organizationIdOrSlug: organization.slug, + snapshotId, + }, + } + ); + + const {data, isPending, isError} = useApiQuery( + [snapshotApiUrl], { staleTime: 0, enabled: !!snapshotId, + // Skip retries on 4xx so error pages render instantly + retry: (count, err) => count < 3 && (!err?.status || err.status >= 500), refetchInterval: query => { const state = query.state.data?.[0]?.comparison_run_info?.state; return state === ComparisonState.PENDING || state === ComparisonState.PROCESSING @@ -381,9 +383,12 @@ export default function SnapshotsPage() { return ( - - {t('Unable to load snapshot data.')} - + ); @@ -392,21 +397,17 @@ export default function SnapshotsPage() { return ( - + - - + diff --git a/static/app/views/preprod/types/snapshotTypes.ts b/static/app/views/preprod/types/snapshotTypes.ts index 81116f241d7c48..fa9dbfb9480e3c 100644 --- a/static/app/views/preprod/types/snapshotTypes.ts +++ b/static/app/views/preprod/types/snapshotTypes.ts @@ -23,6 +23,21 @@ export interface SnapshotComparisonRunInfo { state?: ComparisonState; } +export interface SnapshotApprover { + source: 'sentry' | 'github'; + approved_at?: string | null; + avatar_url?: string | null; + email?: string | null; + id?: string | null; + name?: string | null; + username?: string | null; +} + +export interface SnapshotApprovalInfo { + approvers: SnapshotApprover[]; + status: 'approved' | 'requires_approval'; +} + export interface SnapshotDetailsApiResponse { comparison_type: 'solo' | 'diff'; head_artifact_id: string; @@ -34,6 +49,8 @@ export interface SnapshotDetailsApiResponse { comparison_run_info?: SnapshotComparisonRunInfo | null; + approval_info?: SnapshotApprovalInfo | null; + // Diff fields added: SnapshotImage[]; added_count: number; diff --git a/static/app/views/settings/organizationIntegrations/directEnableButton.spec.tsx b/static/app/views/settings/organizationIntegrations/directEnableButton.spec.tsx new file mode 100644 index 00000000000000..cf6b89e08779da --- /dev/null +++ b/static/app/views/settings/organizationIntegrations/directEnableButton.spec.tsx @@ -0,0 +1,58 @@ +import {OrganizationFixture} from 'sentry-fixture/organization'; + +import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary'; + +import {DirectEnableButton} from 'sentry/views/settings/organizationIntegrations/directEnableButton'; + +describe('DirectEnableButton', () => { + const organization = OrganizationFixture(); + + const defaultProps = { + providerSlug: 'github_copilot', + userHasAccess: true, + buttonProps: { + size: 'sm' as const, + priority: 'primary' as const, + }, + }; + + it('renders Enable Integration button', () => { + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/integrations/direct-enable/github_copilot/`, + method: 'POST', + body: {provider: {key: 'github_copilot'}}, + }); + + render(, {organization}); + + expect(screen.getByRole('button', {name: 'Enable Integration'})).toBeInTheDocument(); + }); + + it('calls the direct-enable endpoint on click', async () => { + const mockPost = MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/integrations/direct-enable/github_copilot/`, + method: 'POST', + body: {provider: {key: 'github_copilot'}}, + }); + + render(, {organization}); + + await userEvent.click(screen.getByRole('button', {name: 'Enable Integration'})); + + await waitFor(() => expect(mockPost).toHaveBeenCalledTimes(1)); + }); + + it('disables button when user does not have access', () => { + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/integrations/direct-enable/github_copilot/`, + method: 'POST', + body: {}, + }); + + render(, { + organization, + }); + + expect(screen.getByRole('button', {name: 'Enable Integration'})).toBeDisabled(); + }); +}); diff --git a/static/app/views/settings/organizationIntegrations/directEnableButton.tsx b/static/app/views/settings/organizationIntegrations/directEnableButton.tsx new file mode 100644 index 00000000000000..7c43067cee2044 --- /dev/null +++ b/static/app/views/settings/organizationIntegrations/directEnableButton.tsx @@ -0,0 +1,72 @@ +import {useMutation} from '@tanstack/react-query'; + +import {Button} from '@sentry/scraps/button'; +import {Tooltip} from '@sentry/scraps/tooltip'; + +import {addErrorMessage} from 'sentry/actionCreators/indicator'; +import {t} from 'sentry/locale'; +import {getApiUrl} from 'sentry/utils/api/getApiUrl'; +import {fetchMutation, useQueryClient} from 'sentry/utils/queryClient'; +import {useOrganization} from 'sentry/utils/useOrganization'; + +import type {AddIntegrationButton} from './addIntegrationButton'; + +interface DirectEnableButtonProps { + buttonProps: Pick< + React.ComponentProps, + 'size' | 'priority' | 'disabled' | 'style' | 'data-test-id' | 'icon' | 'buttonText' + >; + providerSlug: string; + userHasAccess: boolean; +} + +export function DirectEnableButton({ + providerSlug, + buttonProps, + userHasAccess, +}: DirectEnableButtonProps) { + const organization = useOrganization(); + const queryClient = useQueryClient(); + + const {mutate: enable, isPending} = useMutation({ + mutationFn: () => + fetchMutation({ + url: `/organizations/${organization.slug}/integrations/direct-enable/${providerSlug}/`, + method: 'POST', + data: {}, + }), + onSuccess: () => { + queryClient.invalidateQueries({ + queryKey: [ + getApiUrl(`/organizations/$organizationIdOrSlug/integrations/`, { + path: {organizationIdOrSlug: organization.slug}, + }), + ], + }); + queryClient.invalidateQueries({ + queryKey: [ + getApiUrl(`/organizations/$organizationIdOrSlug/config/integrations/`, { + path: {organizationIdOrSlug: organization.slug}, + }), + ], + }); + }, + onError: () => addErrorMessage(t('Failed to enable integration.')), + }); + + return ( + + + + ); +} diff --git a/static/app/views/settings/organizationIntegrations/installedIntegration.spec.tsx b/static/app/views/settings/organizationIntegrations/installedIntegration.spec.tsx new file mode 100644 index 00000000000000..7e0926f302a6d7 --- /dev/null +++ b/static/app/views/settings/organizationIntegrations/installedIntegration.spec.tsx @@ -0,0 +1,58 @@ +import {GitHubIntegrationProviderFixture} from 'sentry-fixture/githubIntegrationProvider'; +import {OrganizationFixture} from 'sentry-fixture/organization'; +import {OrganizationIntegrationsFixture} from 'sentry-fixture/organizationIntegrations'; + +import {render, screen} from 'sentry-test/reactTestingLibrary'; + +import {InstalledIntegration} from 'sentry/views/settings/organizationIntegrations/installedIntegration'; + +describe('InstalledIntegration', () => { + const organization = OrganizationFixture(); + + const defaultProps = { + organization, + integration: OrganizationIntegrationsFixture() as any, + provider: GitHubIntegrationProviderFixture(), + onRemove: jest.fn(), + onDisable: jest.fn(), + trackIntegrationAnalytics: jest.fn(), + }; + + it('shows the Configure button normally', () => { + render(); + + expect(screen.getByRole('button', {name: 'Configure'})).toBeInTheDocument(); + }); + + it('hides the Configure button when directEnable aspect is set', () => { + const provider = GitHubIntegrationProviderFixture({ + metadata: { + ...GitHubIntegrationProviderFixture().metadata, + aspects: {directEnable: true}, + }, + }); + + render(); + + expect(screen.queryByRole('link', {name: 'Configure'})).not.toBeInTheDocument(); + }); + + it('always shows the Uninstall button', () => { + render(); + + expect(screen.getByRole('button', {name: 'Uninstall'})).toBeInTheDocument(); + }); + + it('always shows the Uninstall button when directEnable is set', () => { + const provider = GitHubIntegrationProviderFixture({ + metadata: { + ...GitHubIntegrationProviderFixture().metadata, + aspects: {directEnable: true}, + }, + }); + + render(); + + expect(screen.getByRole('button', {name: 'Uninstall'})).toBeInTheDocument(); + }); +}); diff --git a/static/app/views/settings/organizationIntegrations/installedIntegration.tsx b/static/app/views/settings/organizationIntegrations/installedIntegration.tsx index 9bdd5295a3518d..aaf1ddd4534a6b 100644 --- a/static/app/views/settings/organizationIntegrations/installedIntegration.tsx +++ b/static/app/views/settings/organizationIntegrations/installedIntegration.tsx @@ -153,15 +153,17 @@ export class InstalledIntegration extends Component { size="sm" /> )} - } - disabled={!allowMemberConfiguration && !canConfigure} - to={`/settings/${organization.slug}/integrations/${provider.key}/${integration.id}/`} - data-test-id="integration-configure-button" - > - {t('Configure')} - + {!provider.metadata.aspects?.directEnable && ( + } + disabled={!allowMemberConfiguration && !canConfigure} + to={`/settings/${organization.slug}/integrations/${provider.key}/${integration.id}/`} + data-test-id="integration-configure-button" + > + {t('Configure')} + + )}
diff --git a/static/app/views/settings/organizationIntegrations/integrationButton.spec.tsx b/static/app/views/settings/organizationIntegrations/integrationButton.spec.tsx index 7a68be304d5f12..892917cf7c5b8e 100644 --- a/static/app/views/settings/organizationIntegrations/integrationButton.spec.tsx +++ b/static/app/views/settings/organizationIntegrations/integrationButton.spec.tsx @@ -102,4 +102,31 @@ describe('AddIntegrationButton', () => { await userEvent.click(screen.getByText('Add Installation')); }); + + it('renders Enable Integration button when directEnable aspect is set and canAdd is true', () => { + provider.canAdd = true; + provider.metadata.aspects = {directEnable: true}; + + MockApiClient.addMockResponse({ + url: `/organizations/${org.slug}/integrations/direct-enable/${provider.slug}/`, + method: 'POST', + body: {}, + }); + + render(getComponent(), {organization: org}); + + expect(screen.getByRole('button', {name: 'Enable Integration'})).toBeInTheDocument(); + }); + + it('renders nothing when directEnable aspect is set and canAdd is false (already installed)', () => { + provider.canAdd = false; + provider.metadata.aspects = {directEnable: true}; + + render(getComponent(), {organization: org}); + + expect( + screen.queryByRole('button', {name: 'Enable Integration'}) + ).not.toBeInTheDocument(); + expect(screen.queryByRole('button')).not.toBeInTheDocument(); + }); }); diff --git a/static/app/views/settings/organizationIntegrations/integrationButton.tsx b/static/app/views/settings/organizationIntegrations/integrationButton.tsx index 6080a6e6a7a567..fc6472ee99f4fa 100644 --- a/static/app/views/settings/organizationIntegrations/integrationButton.tsx +++ b/static/app/views/settings/organizationIntegrations/integrationButton.tsx @@ -6,6 +6,7 @@ import {IconOpen} from 'sentry/icons'; import type {Integration} from 'sentry/types/integrations'; import {useOrganization} from 'sentry/utils/useOrganization'; import {AddIntegrationButton} from 'sentry/views/settings/organizationIntegrations/addIntegrationButton'; +import {DirectEnableButton} from 'sentry/views/settings/organizationIntegrations/directEnableButton'; import {IntegrationContext} from 'sentry/views/settings/organizationIntegrations/integrationContext'; import {RequestIntegrationButton} from 'sentry/views/settings/organizationIntegrations/integrationRequest/RequestIntegrationButton'; @@ -44,6 +45,15 @@ export function IntegrationButton({ ); } + if (metadata.aspects.directEnable) { + return provider.canAdd ? ( + + ) : null; + } if (provider.canAdd) { return ( "), @@ -48,6 +49,9 @@ ("ip - v4", "11.21.12.31", ""), ("ip - v6 unspecified", "::", ""), ("ip - v6 loopback", "::1", ""), + ("ip - v6 ULA", "fc00::/7", ""), + ("ip - v6 compressed", "2012:d157::cbe:908:2013", ""), + ("ip - v4 mapped to v6", "::ffff:192.168.1.1", ""), ("ip - v6 full", "1121:0c03:1231:130d:0000:16da:0908:da07", ""), ("ip - double colon object property", "Option::unwrap()", "Option::unwrap()"), ("ip - double colon object property including hex", "Bee::buzz()", "Bee::buzz()"), @@ -278,24 +282,6 @@ def test_experimental_parameterization(name: str, input: str, expected: str) -> "Fee::add() called too early", "() called too early", ), - ( - "ip - v4 mapped to v6", - "::ffff:192.168.1.1", - "", - "..", - ), - ( - "ip - v6 compressed", - "2012:d157::cbe:908:2013", - "", - "::", - ), - ( - "ip - v6 ULA", - "fc00::/7", - "", - "/", - ), ( "json - double quotes", '{"dogs are great": true, "dog_id": "greatdog1231"}', @@ -630,3 +616,95 @@ def test_uses_callback_for_replacement_value() -> None: callback_parameterizer.parameterize(input_str) == "Dog number , # dog" # Callback function was used ) + + +def test_replacement_callback_false_positive_triggers_individual_regex_fallback() -> None: + # `12:31:99` matches the IPv6 regex pattern but isn't a valid IP address, so including it in + # our input should trigger the false positive fallback behavior + real_parameterizer_regexes = parameterizer.compiled_regexes_by_name + real_ip_regex = real_parameterizer_regexes["ip"] + assert real_ip_regex.fullmatch("12:31:99") + assert not is_valid_ip("12:31:99") + + input_str = "1a2b3c4d5e6f 12:31:99" + + # Mock a whole bunch of things, to prove various points: + # + # - To show that we're indeed landing in the false positive fallback block, mock the + # individual regexes' `sub` methods and the tags we attach to the timing metric. + # + # - To show that parameterization runs twice, and that the fallback is necessary, mock the + # regular combo regex's `sub` method, and capture its return value. + # + # - To show we're counting correctly, even though we're parameterizing twice, mock the counter + # metric. + # + # Note: Mocking the `sub` methods is made more complicated by the fact that regex objects' `sub` + # attributes are read-only, and therefore can't be directly replaced by pytest. Instead, we have + # to use nested mocks to replace the entire regex object. And for the timer tags, we need to + # mock the tags dictionary itself (in other words, what's returned by the timer's context + # manager's `__enter__` method) - rather than just asserting on the timer's call args - because + # when the initial `metrics.timer` call happens, the `false_positive` tag hasn't yet been set. + combo_regex_sub_method_return_values: list[str] = [] + metrics_timer_tags: dict[str, bool] = {} + mock_hex_regex = MagicMock(sub=MagicMock(side_effect=real_parameterizer_regexes["hex"].sub)) + mock_ip_regex = MagicMock(sub=MagicMock(side_effect=real_parameterizer_regexes["ip"].sub)) + mock_int_regex = MagicMock(sub=MagicMock(side_effect=real_parameterizer_regexes["int"].sub)) + mock_metrics_timer_context_manager = MagicMock( + __enter__=MagicMock(return_value=metrics_timer_tags) + ) + mock_combo_regex = MagicMock( + sub=MagicMock( + side_effect=capture_results( + parameterizer.combined_regex.sub, combo_regex_sub_method_return_values + ) + ) + ) + + with ( + patch.dict( + parameterizer.compiled_regexes_by_name, + {"hex": mock_hex_regex, "ip": mock_ip_regex, "int": mock_int_regex}, + ), + patch( + "sentry.grouping.parameterization.metrics.timer", + return_value=mock_metrics_timer_context_manager, + ), + patch.object(parameterizer, "combined_regex", mock_combo_regex), + patch("sentry.grouping.parameterization.metrics.incr") as mock_metrics_incr, + ): + # First check that the fallback behavior produces the desired result - even though the IP + # pattern matches, we still go on to find the int pattern match + assert parameterizer.parameterize(input_str) == " ::" + + # We can see that it was indeed the fallback saving us from getting the wrong answer by + # checking what's returned by the combo regex, before the fallback runs + assert combo_regex_sub_method_return_values[0] == " 12:31:99" + + # Check that parameterization ran twice, once the regular way and once using the fallback. + # (We can see we landed in the fallback both because the individual regexes' `sub` methods + # were called and by looking at the tags on the timing metric.) + mock_combo_regex.sub.assert_called() + mock_hex_regex.sub.assert_called() + mock_ip_regex.sub.assert_called() + mock_int_regex.sub.assert_called() + assert metrics_timer_tags == {"false_positive": True, "changed": True} + + # Even though the parameterization ran twice, the counts (as reflected in the count metric + # calls) are still correct - one hex param, three int params, and no ip params + expected_count_metric_calls = [("hex", 1), ("int", 3)] + for key, amount in expected_count_metric_calls: + mock_metrics_incr.assert_any_call( + "grouping.value_parameterized", + amount=amount, + tags={"key": key}, + ) + assert ( + count_matching_calls( + mock_metrics_incr, + "grouping.value_parameterized", + amount=ANY, + tags={"key": "ip"}, + ) + == 0 + ) diff --git a/tests/sentry/seer/explorer/test_tools.py b/tests/sentry/seer/explorer/test_tools.py index a177cef6e5a9cc..0b87b5ae009c43 100644 --- a/tests/sentry/seer/explorer/test_tools.py +++ b/tests/sentry/seer/explorer/test_tools.py @@ -2076,7 +2076,7 @@ def test_no_start_end_uses_group_date_range(self, mock_api_client): # Validate return value: (data, stats_period, interval). assert result is not None timeseries_data, returned_period, returned_interval = result - _validate_event_timeseries(timeseries_data) + _validate_event_timeseries(timeseries_data, expected_total=2) assert returned_period == stats_period assert returned_interval == interval @@ -2107,6 +2107,11 @@ def test_with_start_end_uses_explicit_range(self, mock_api_client): data["exception"] = {"values": [{"type": "Exception", "value": "Test exception"}]} self.store_event(data=data, project_id=self.project.id) + # Out of range event + data = load_data("python", timestamp=start - timedelta(minutes=1)) + data["exception"] = {"values": [{"type": "Exception", "value": "Test exception"}]} + self.store_event(data=data, project_id=self.project.id) + group = event.group assert isinstance(group, Group) @@ -2132,7 +2137,7 @@ def test_with_start_end_uses_explicit_range(self, mock_api_client): # Validate return value: (data, stats_period, interval). assert result is not None timeseries_data, returned_period, returned_interval = result - _validate_event_timeseries(timeseries_data) + _validate_event_timeseries(timeseries_data, expected_total=2) assert returned_period == stats_period assert returned_interval == interval