Merge branch 'master' into brendan/metrics-tooltip

Author: brendanhsentry

.github/workflows/scripts/compute-sentry-selected-tests.py

@@ -86,6 +86,11 @@
     re.compile(r"^tests/(acceptance|apidocs|js|tools)/"),
 ]
 
+# Tests that should always be run even if not explicitly selected.
+ALWAYS_RUN_TESTS: set[str] = {
+    "tests/sentry/taskworker/test_config.py",
+}
+
 
 def _is_test(path: str) -> bool:
     return any(path.startswith(d) for d in TEST_DIRS)
@@ -225,6 +230,9 @@ def main() -> int:
                 print(f"Including {len(existing_changed)} directly changed test files")
                 affected_test_files.update(existing_changed)
 
+            # Always run these tests
+            affected_test_files.update(ALWAYS_RUN_TESTS)
+
     # Filter to sentry tests only (drop any getsentry tests from coverage)
     affected_test_files = {f for f in affected_test_files if _is_test(f)}
 

.github/workflows/scripts/test_compute_sentry_selected_tests.py

@@ -19,7 +19,7 @@
 sys.modules["compute_sentry_selected_tests"] = _mod
 _spec.loader.exec_module(_mod)
 
-from compute_sentry_selected_tests import _query_coverage, main
+from compute_sentry_selected_tests import ALWAYS_RUN_TESTS, _query_coverage, main
 
 
 def _create_coverage_db(path: str, file_to_contexts: dict[str, list[str]]) -> None:
@@ -178,8 +178,9 @@ def test_selective_returns_matched_tests(self, tmp_path):
 
         gh = gh_output.read_text()
         assert "has-selected-tests=true" in gh
-        assert "test-count=1" in gh
-        assert output.read_text().strip() == "tests/sentry/test_org.py"
+        assert f"test-count={1 + len(ALWAYS_RUN_TESTS)}" in gh
+        expected_output = sorted({"tests/sentry/test_org.py"} | ALWAYS_RUN_TESTS)
+        assert output.read_text().splitlines() == expected_output
 
     def test_getsentry_tests_filtered_out(self, tmp_path):
         """Coverage may return getsentry tests — they should be filtered."""
@@ -211,8 +212,9 @@ def test_getsentry_tests_filtered_out(self, tmp_path):
                 {"GITHUB_OUTPUT": str(gh_output)},
             )
 
-        assert "test-count=1" in gh_output.read_text()
-        assert output.read_text().strip() == "tests/sentry/test_org.py"
+        assert f"test-count={1 + len(ALWAYS_RUN_TESTS)}" in gh_output.read_text()
+        expected_output = sorted({"tests/sentry/test_org.py"} | ALWAYS_RUN_TESTS)
+        assert output.read_text().splitlines() == expected_output
 
     def test_changed_test_file_included(self, tmp_path):
         db_path = tmp_path / "coverage.db"
@@ -236,7 +238,7 @@ def test_changed_test_file_included(self, tmp_path):
 
         gh = gh_output.read_text()
         assert "has-selected-tests=true" in gh
-        assert "test-count=1" in gh
+        assert f"test-count={1 + len(ALWAYS_RUN_TESTS)}" in gh
 
     def test_excluded_test_dirs_skipped(self, tmp_path):
         """Tests in acceptance/apidocs/js/tools should not be selected."""
@@ -257,10 +259,10 @@ def test_excluded_test_dirs_skipped(self, tmp_path):
                 {"GITHUB_OUTPUT": str(gh_output)},
             )
 
-        assert "test-count=0" in gh_output.read_text()
+        assert f"test-count={len(ALWAYS_RUN_TESTS)}" in gh_output.read_text()
 
-    def test_zero_tests_signals_selective_applied(self, tmp_path):
-        """0 tests after filtering should signal 'run nothing', not full suite."""
+    def test_zero_coverage_matches_still_runs_always_run_tests(self, tmp_path):
+        """No coverage matches should still run ALWAYS_RUN_TESTS, not the full suite."""
         db_path = tmp_path / "coverage.db"
         _create_coverage_db(str(db_path), {})
         output = tmp_path / "output.txt"
@@ -282,8 +284,8 @@ def test_zero_tests_signals_selective_applied(self, tmp_path):
 
         gh = gh_output.read_text()
         assert "has-selected-tests=true" in gh
-        assert "test-count=0" in gh
-        assert output.read_text() == ""
+        assert f"test-count={len(ALWAYS_RUN_TESTS)}" in gh
+        assert set(output.read_text().splitlines()) == ALWAYS_RUN_TESTS
 
     def test_renamed_file_queries_old_path(self, tmp_path):
         """When a file is renamed, the old path should be queried against the coverage DB."""
@@ -319,8 +321,9 @@ def test_renamed_file_queries_old_path(self, tmp_path):
 
         gh = gh_output.read_text()
         assert "has-selected-tests=true" in gh
-        assert "test-count=1" in gh
-        assert output.read_text().strip() == "tests/sentry/test_old_name.py"
+        assert f"test-count={1 + len(ALWAYS_RUN_TESTS)}" in gh
+        expected_output = sorted({"tests/sentry/test_old_name.py"} | ALWAYS_RUN_TESTS)
+        assert output.read_text().splitlines() == expected_output
 
     def test_renamed_file_without_previous_misses_coverage(self, tmp_path):
         """Without --previous-filenames, a renamed file gets no coverage hits."""
@@ -349,7 +352,7 @@ def test_renamed_file_without_previous_misses_coverage(self, tmp_path):
 
         gh = gh_output.read_text()
         assert "has-selected-tests=true" in gh
-        assert "test-count=0" in gh
+        assert f"test-count={len(ALWAYS_RUN_TESTS)}" in gh
 
     def test_missing_db_returns_error(self):
         ret = _run(["--coverage-db", "/nonexistent/coverage.db", "--changed-files", "foo.py"])

src/sentry/notifications/notification_action/types.py

@@ -35,7 +35,7 @@
 )
 from sentry.types.activity import ActivityType
 from sentry.types.rules import RuleFuture
-from sentry.workflow_engine.models import Action, AlertRuleWorkflow, Detector
+from sentry.workflow_engine.models import Action, AlertRuleWorkflow, Detector, Workflow
 from sentry.workflow_engine.types import ActionInvocation, DetectorPriorityLevel, WorkflowEventData
 from sentry.workflow_engine.typings.notification_action import (
     ACTION_FIELD_MAPPINGS,
@@ -201,7 +201,19 @@ def create_rule_instance_from_action(
         workflow_id = getattr(action, "workflow_id", None)
         rule_id = None
 
-        label = detector.name
+        label = None
+        # Attempt to query the workflow name for non-test notifications.
+        if workflow_id is not None and workflow_id != TEST_NOTIFICATION_ID:
+            try:
+                workflow = Workflow.objects.get(id=workflow_id)
+                label = workflow.name
+            except Workflow.DoesNotExist:
+                # If the workflow no longer exists, bail and use detector name
+                # as a fallback.
+                pass
+
+        if label is None:
+            label = detector.name
         # Build link to the rule if it exists, otherwise build link to the workflow.
         # FE will handle redirection if necessary from rule -> workflow
 

static/app/components/noProjectMessage.tsx

@@ -9,8 +9,8 @@ import * as Layout from 'sentry/components/layouts/thirds';
 import {t} from 'sentry/locale';
 import type {Organization} from 'sentry/types/organization';
 import {useCanCreateProject} from 'sentry/utils/useCanCreateProject';
+import {useHasProjectAccess} from 'sentry/utils/useHasProjectAccess';
 import {useProjects} from 'sentry/utils/useProjects';
-import {useUser} from 'sentry/utils/useUser';
 import {makeProjectsPathname} from 'sentry/views/projects/pathname';
 
 type Props = {
@@ -24,17 +24,15 @@ export function NoProjectMessage({
   organization,
   superuserNeedsToBeProjectMember,
 }: Props) {
-  const user = useUser();
-  const {projects, initiallyLoaded: projectsLoaded} = useProjects();
+  const {projects} = useProjects();
+  const {hasProjectAccess, projectsLoaded} = useHasProjectAccess({
+    superuserNeedsToBeProjectMember,
+  });
 
   const canUserCreateProject = useCanCreateProject();
   const canJoinTeam = organization.access.includes('team:read');
 
   const orgHasProjects = !!projects?.length;
-  const hasProjectAccess =
-    user.isSuperuser && !superuserNeedsToBeProjectMember
-      ? !!projects?.some(p => p.hasAccess)
-      : !!projects?.some(p => p.isMember && p.hasAccess);
 
   if (hasProjectAccess || !projectsLoaded) {
     return <Fragment>{children}</Fragment>;

static/app/utils/useHasProjectAccess.spec.tsx

@@ -0,0 +1,32 @@
+import {ProjectFixture} from 'sentry-fixture/project';
+
+import {renderHookWithProviders} from 'sentry-test/reactTestingLibrary';
+
+import {ConfigStore} from 'sentry/stores/configStore';
+import {ProjectsStore} from 'sentry/stores/projectsStore';
+
+import {useHasProjectAccess} from './useHasProjectAccess';
+
+describe('useHasProjectAccess', () => {
+  beforeEach(() => {
+    ProjectsStore.reset();
+    ConfigStore.set('user', {...ConfigStore.get('user'), isSuperuser: false});
+  });
+
+  it('returns false when there are no projects', () => {
+    ProjectsStore.loadInitialData([]);
+
+    const {result} = renderHookWithProviders(() => useHasProjectAccess());
+
+    expect(result.current.hasProjectAccess).toBe(false);
+    expect(result.current.projectsLoaded).toBe(true);
+  });
+
+  it('returns true when user is a member of a project with access', () => {
+    ProjectsStore.loadInitialData([ProjectFixture({hasAccess: true, isMember: true})]);
+
+    const {result} = renderHookWithProviders(() => useHasProjectAccess());
+
+    expect(result.current.hasProjectAccess).toBe(true);
+  });
+});

static/app/utils/useHasProjectAccess.tsx

@@ -0,0 +1,25 @@
+import {useProjects} from 'sentry/utils/useProjects';
+import {useUser} from 'sentry/utils/useUser';
+
+type Options = {
+  /**
+   * When true, superusers must also be a project member to count as having access.
+   */
+  superuserNeedsToBeProjectMember?: boolean;
+};
+
+/**
+ * Returns whether the current user has access to at least one project,
+ * and whether the project list has finished loading.
+ */
+export function useHasProjectAccess(options?: Options) {
+  const user = useUser();
+  const {projects, initiallyLoaded: projectsLoaded} = useProjects();
+
+  const hasProjectAccess =
+    user.isSuperuser && !options?.superuserNeedsToBeProjectMember
+      ? !!projects?.some(p => p.hasAccess)
+      : !!projects?.some(p => p.isMember && p.hasAccess);
+
+  return {hasProjectAccess, projectsLoaded};
+}

static/app/views/dashboards/hooks/useGetStarredDashboards.tsx

@@ -2,6 +2,7 @@ import type {Organization} from 'sentry/types/organization';
 import {getApiUrl} from 'sentry/utils/api/getApiUrl';
 import type {ApiQueryKey} from 'sentry/utils/queryClient';
 import {useApiQuery} from 'sentry/utils/queryClient';
+import {useHasProjectAccess} from 'sentry/utils/useHasProjectAccess';
 import {useOrganization} from 'sentry/utils/useOrganization';
 import type {DashboardListItem} from 'sentry/views/dashboards/types';
 
@@ -29,7 +30,10 @@ export function getStarredDashboardsQueryKey(organization: Organization): ApiQue
 
 export function useGetStarredDashboards() {
   const organization = useOrganization();
+  const {hasProjectAccess, projectsLoaded} = useHasProjectAccess();
+
   return useApiQuery<DashboardListItem[]>(getStarredDashboardsQueryKey(organization), {
     staleTime: Infinity,
+    enabled: hasProjectAccess || !projectsLoaded,
   });
 }

static/app/views/dashboards/manage/index.spec.tsx

@@ -112,6 +112,25 @@ describe('Dashboards > Detail', () => {
     ).toBeInTheDocument();
   });
 
+  it('does not fetch dashboards when there are no projects', async () => {
+    act(() => ProjectsStore.loadInitialData([]));
+
+    const dashboardsRequest = MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/dashboards/',
+      body: [],
+    });
+
+    render(<ManageDashboards />, {
+      organization: mockAuthorizedOrg,
+    });
+
+    expect(
+      await screen.findByText('You need at least one project to use this view')
+    ).toBeInTheDocument();
+
+    expect(dashboardsRequest).not.toHaveBeenCalled();
+  });
+
   it('creates new dashboard', async () => {
     const org = OrganizationFixture({features: FEATURES});
     const mockNavigate = jest.fn();

static/app/views/dashboards/manage/index.tsx

@@ -39,6 +39,7 @@ import {decodeScalar} from 'sentry/utils/queryString';
 import {scheduleMicroTask} from 'sentry/utils/scheduleMicroTask';
 import {normalizeUrl} from 'sentry/utils/url/normalizeUrl';
 import {useApi} from 'sentry/utils/useApi';
+import {useHasProjectAccess} from 'sentry/utils/useHasProjectAccess';
 import {useLocalStorageState} from 'sentry/utils/useLocalStorageState';
 import {useLocation} from 'sentry/utils/useLocation';
 import {useNavigate} from 'sentry/utils/useNavigate';
@@ -189,6 +190,8 @@ function ManageDashboards() {
     columnCount: DASHBOARD_GRID_DEFAULT_NUM_COLUMNS,
   });
 
+  const {hasProjectAccess, projectsLoaded} = useHasProjectAccess();
+
   const sortOptions = getSortOptions({
     organization,
     dashboardsLayout,
@@ -222,10 +225,12 @@ function ManageDashboards() {
     ],
     {
       staleTime: 0,
-      enabled: !(
-        organization.features.includes('dashboards-starred-reordering') &&
-        dashboardsLayout === TABLE
-      ),
+      enabled:
+        (hasProjectAccess || !projectsLoaded) &&
+        !(
+          organization.features.includes('dashboards-starred-reordering') &&
+          dashboardsLayout === TABLE
+        ),
     }
   );
 
@@ -257,6 +262,7 @@ function ManageDashboards() {
     cursor: decodeScalar(location.query[OWNED_CURSOR_KEY], ''),
     sort: getActiveSort()!.value,
     enabled:
+      (hasProjectAccess || !projectsLoaded) &&
       organization.features.includes('dashboards-starred-reordering') &&
       dashboardsLayout === TABLE,
   });

tests/sentry/notifications/notification_action/test_issue_alert_registry_handlers.py

@@ -25,7 +25,7 @@
     BaseIssueAlertHandler,
     TicketingIssueAlertHandler,
 )
-from sentry.notifications.types import ActionTargetType, FallthroughChoiceType
+from sentry.notifications.types import TEST_NOTIFICATION_ID, ActionTargetType, FallthroughChoiceType
 from sentry.testutils.helpers.data_blobs import (
     AZURE_DEVOPS_ACTION_DATA_BLOBS,
     EMAIL_ACTION_DATA_BLOBS,
@@ -168,7 +168,7 @@ def test_create_rule_instance_from_action_with_workflow_only(self) -> None:
         assert rule.environment_id is not None
         assert self.workflow.environment is not None
         assert rule.environment_id == self.workflow.environment.id
-        assert rule.label == self.detector.name
+        assert rule.label == self.workflow.name
         assert rule.data == {
             "actions": [
                 {
@@ -183,6 +183,51 @@ def test_create_rule_instance_from_action_with_workflow_only(self) -> None:
         assert rule.status == ObjectStatus.ACTIVE
         assert rule.source == RuleSource.ISSUE
 
+    def test_create_rule_instance_from_action_deleted_workflow_falls_back_to_detector_name(
+        self,
+    ) -> None:
+        """Test that label falls back to detector.name when the workflow no longer exists"""
+        workflow_id = self.workflow.id
+        self.workflow.delete()
+        rule = self.handler.create_rule_instance_from_action(
+            self.action, self.detector, self.event_data
+        )
+
+        assert isinstance(rule, Rule)
+        assert rule.label == self.detector.name
+        assert rule.data == {
+            "actions": [
+                {
+                    "id": "sentry.integrations.discord.notify_action.DiscordNotifyServiceAction",
+                    "server": "1234567890",
+                    "channel_id": "channel456",
+                    "tags": "environment,user,my_tag",
+                    "workflow_id": workflow_id,
+                }
+            ]
+        }
+
+    def test_create_rule_instance_from_action_with_test_notification_id(self) -> None:
+        """Test that Workflow lookup is skipped for test notifications, falling back to detector name"""
+        self.action.workflow_id = TEST_NOTIFICATION_ID
+        rule = self.handler.create_rule_instance_from_action(
+            self.action, self.detector, self.event_data
+        )
+
+        assert isinstance(rule, Rule)
+        assert rule.label == self.detector.name
+        assert rule.data == {
+            "actions": [
+                {
+                    "id": "sentry.integrations.discord.notify_action.DiscordNotifyServiceAction",
+                    "server": "1234567890",
+                    "channel_id": "channel456",
+                    "tags": "environment,user,my_tag",
+                    "legacy_rule_id": TEST_NOTIFICATION_ID,
+                }
+            ],
+        }
+
     def test_create_rule_instance_from_action_no_environment(self) -> None:
         """Test that create_rule_instance_from_action creates a Rule with correct attributes"""
         self.create_workflow()