perf(issue-details): Get selected environments from query string (#49590)

Closes https://github.com/getsentry/sentry/issues/49281

Previously:

- `<GroupDetails />` waited for `PageFiltersStore` to have loaded the
selection and have `isGlobalSlectionReady: true` before making the main
request
- `<GroupDetails />` made a request to fetch environment info in
OrganizationEnvironmentsStore which was used in `<GroupEventDetails />`.
If the event was loaded but environments were not, this was blocked by a
loading spinner.

Now the array of environments are collected from the query string in the
URL so there is no need to block anything. This also removes an extra
request to fetch environments and a store that is no longer used.

static/app/actionCreators/environments.tsx

@@ -1,28 +0,0 @@
-import {Client} from 'sentry/api';
-import OrganizationEnvironmentsStore from 'sentry/stores/organizationEnvironmentsStore';
-
-/**
- * Fetches all environments for an organization
- *
- * @param organizationSlug The organization slug
- */
-export async function fetchOrganizationEnvironments(
-  api: Client,
-  organizationSlug: string
-) {
-  OrganizationEnvironmentsStore.onFetchEnvironments();
-  try {
-    const environments = await api.requestPromise(
-      `/organizations/${organizationSlug}/environments/`
-    );
-    if (!environments) {
-      OrganizationEnvironmentsStore.onFetchEnvironmentsError(
-        new Error('retrieved environments is falsey')
-      );
-      return;
-    }
-    OrganizationEnvironmentsStore.onFetchEnvironmentsSuccess(environments);
-  } catch (err) {
-    OrganizationEnvironmentsStore.onFetchEnvironmentsError(err);
-  }
-}

static/app/components/errors/groupEventDetailsLoadingError.tsx

@@ -2,10 +2,9 @@ import DetailedError from 'sentry/components/errors/detailedError';
 import List from 'sentry/components/list';
 import ListItem from 'sentry/components/list/listItem';
 import {t} from 'sentry/locale';
-import {Environment} from 'sentry/types';
 
 type Props = {
-  environments: Environment[];
+  environments: string[];
   onRetry?: (e: React.MouseEvent) => void;
 };
 

static/app/components/group/releaseStats.tsx

@@ -10,13 +10,13 @@ import {Tooltip} from 'sentry/components/tooltip';
 import {IconQuestion} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import {CurrentRelease, Environment, Group, Organization, Project} from 'sentry/types';
+import {CurrentRelease, Group, Organization, Project} from 'sentry/types';
 import getDynamicText from 'sentry/utils/getDynamicText';
 
 type Props = {
   allEnvironments: Group | undefined;
   currentRelease: CurrentRelease | undefined;
-  environments: Environment[];
+  environments: string[];
   group: Group | undefined;
   organization: Organization;
   project: Project;
@@ -30,17 +30,14 @@ function GroupReleaseStats({
   group,
   currentRelease,
 }: Props) {
-  const environment =
-    environments.length > 0
-      ? environments.map(env => env.displayName).join(', ')
-      : undefined;
+  const environment = environments.length > 0 ? environments.join(', ') : undefined;
   const environmentLabel = environment ? environment : t('All Environments');
 
   const shortEnvironmentLabel =
     environments.length > 1
       ? t('selected environments')
       : environments.length === 1
-      ? environments[0].displayName
+      ? environments[0]
       : undefined;
 
   const projectId = project.id;

static/app/components/group/tagFacets/index.tsx

@@ -11,7 +11,7 @@ import {Tooltip} from 'sentry/components/tooltip';
 import {IconQuestion} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import {Environment, Event, Organization, Project} from 'sentry/types';
+import {Event, Organization, Project} from 'sentry/types';
 import {formatVersion} from 'sentry/utils/formatters';
 import {appendTagCondition} from 'sentry/utils/queryString';
 import {useLocation} from 'sentry/utils/useLocation';
@@ -73,7 +73,7 @@ export function TAGS_FORMATTER(tagsData: Record<string, GroupTagResponseItem>) {
 }
 
 type Props = {
-  environments: Environment[];
+  environments: string[];
   groupId: string;
   project: Project;
   tagKeys: string[];
@@ -94,7 +94,7 @@ export default function TagFacets({
 
   const {isLoading, isError, data, refetch} = useFetchIssueTagsForDetailsPage({
     groupId,
-    environment: environments.map(({name}) => name),
+    environment: environments,
   });
 
   const tagsData = useMemo(() => {

static/app/stores/organizationEnvironmentsStore.spec.jsx

@@ -1,52 +0,0 @@
-import OrganizationEnvironmentsStore from 'sentry/stores/organizationEnvironmentsStore';
-
-describe('OrganizationEnvironmentsStore', function () {
-  beforeEach(() => {
-    OrganizationEnvironmentsStore.init();
-  });
-
-  it('get()', function () {
-    expect(OrganizationEnvironmentsStore.getState()).toEqual({
-      environments: null,
-      error: null,
-    });
-  });
-
-  it('loads data from a fetch', async function () {
-    OrganizationEnvironmentsStore.onFetchEnvironmentsSuccess(TestStubs.Environments());
-
-    await tick();
-
-    const {environments} = OrganizationEnvironmentsStore.getState();
-
-    expect(environments).toHaveLength(3);
-    expect(environments.map(env => env.name)).toEqual([
-      'production',
-      'staging',
-      'STAGING',
-    ]);
-    expect(environments.map(env => env.displayName)).toEqual([
-      'production',
-      'staging',
-      'STAGING',
-    ]);
-  });
-
-  it('has the correct loading state', function () {
-    OrganizationEnvironmentsStore.onFetchEnvironments();
-
-    const {environments, error} = OrganizationEnvironmentsStore.getState();
-
-    expect(environments).toBeNull();
-    expect(error).toBeNull();
-  });
-
-  it('has the correct error state', function () {
-    OrganizationEnvironmentsStore.onFetchEnvironmentsError(Error('bad'));
-
-    const {environments, error} = OrganizationEnvironmentsStore.getState();
-
-    expect(environments).toBeNull();
-    expect(error).not.toBeNull();
-  });
-});

static/app/stores/organizationEnvironmentsStore.tsx

@@ -1,73 +0,0 @@
-import {createStore} from 'reflux';
-
-import {Environment} from 'sentry/types';
-import {getDisplayName, getUrlRoutingName} from 'sentry/utils/environment';
-
-import {CommonStoreDefinition} from './types';
-
-type EnhancedEnvironment = Environment & {
-  displayName: string;
-  urlRoutingName: string;
-};
-
-type State = {
-  environments: EnhancedEnvironment[] | null;
-  error: Error | null;
-};
-
-interface OrganizationEnvironmentsStoreDefinition extends CommonStoreDefinition<State> {
-  init(): void;
-  onFetchEnvironments(): void;
-  onFetchEnvironmentsError(error: Error): void;
-  onFetchEnvironmentsSuccess(environments: Environment[]): void;
-  state: State;
-}
-
-const storeConfig: OrganizationEnvironmentsStoreDefinition = {
-  state: {
-    environments: null,
-    error: null,
-  },
-
-  init() {
-    // XXX: Do not use `this.listenTo` in this store. We avoid usage of reflux
-    // listeners due to their leaky nature in tests.
-
-    this.state = {environments: null, error: null};
-  },
-
-  makeEnvironment(item: Environment): EnhancedEnvironment {
-    return {
-      id: item.id,
-      name: item.name,
-      get displayName() {
-        return getDisplayName(item);
-      },
-      get urlRoutingName() {
-        return getUrlRoutingName(item);
-      },
-    };
-  },
-
-  onFetchEnvironments() {
-    this.state = {environments: null, error: null};
-    this.trigger(this.state);
-  },
-
-  onFetchEnvironmentsSuccess(environments) {
-    this.state = {error: null, environments: environments.map(this.makeEnvironment)};
-    this.trigger(this.state);
-  },
-
-  onFetchEnvironmentsError(error) {
-    this.state = {error, environments: null};
-    this.trigger(this.state);
-  },
-
-  getState() {
-    return this.state;
-  },
-};
-
-const OrganizationEnvironmentsStore = createStore(storeConfig);
-export default OrganizationEnvironmentsStore;

static/app/views/issueDetails/groupDetails.spec.jsx

@@ -19,7 +19,6 @@ describe('groupDetails', () => {
   const group = TestStubs.Group({issueCategory: IssueCategory.ERROR});
   const event = TestStubs.Event();
   const project = TestStubs.Project({teams: [TestStubs.Team()]});
-  const selection = {environments: []};
 
   const routes = [
     {path: '/', childRoutes: [], component: null},
@@ -37,20 +36,22 @@ describe('groupDetails', () => {
     },
   ];
 
-  const {organization, router, routerContext} = initializeOrg({
-    project,
-    router: {
-      location: {
-        pathname: `/organizations/org-slug/issues/${group.id}/`,
-        query: {},
-        search: '?foo=bar',
-        hash: '#hash',
-      },
-      params: {
-        groupId: group.id,
-      },
-      routes,
+  const initRouter = {
+    location: {
+      pathname: `/organizations/org-slug/issues/${group.id}/`,
+      query: {},
+      search: '?foo=bar',
+      hash: '#hash',
     },
+    params: {
+      groupId: group.id,
+    },
+    routes,
+  };
+
+  const defaultInit = initializeOrg({
+    project,
+    router: initRouter,
   });
 
   function MockComponent({group: groupProp, environments, eventError}) {
@@ -64,23 +65,22 @@ describe('groupDetails', () => {
     );
   }
 
-  const createWrapper = (props = {selection}, org = organization) => {
+  const createWrapper = (init = defaultInit) => {
     return render(
       <GroupDetails
-        {...router}
-        router={router}
-        selection={props.selection}
-        organization={org}
+        {...init.router}
+        router={init.router}
+        organization={init.organization}
       >
         <MockComponent />
       </GroupDetails>,
-      {context: routerContext, organization: org, router}
+      {context: init.routerContext, organization: init.organization, router: init.router}
     );
   };
 
   beforeEach(() => {
-    OrganizationStore.onUpdate(organization);
-    act(() => ProjectsStore.loadInitialData(organization.projects));
+    OrganizationStore.onUpdate(defaultInit.organization);
+    act(() => ProjectsStore.loadInitialData(defaultInit.organization.projects));
 
     MockApiClient.addMockResponse({
       url: `/issues/${group.id}/`,
@@ -109,7 +109,7 @@ describe('groupDetails', () => {
       body: {firstRelease: group.firstRelease, lastRelease: group.lastRelease},
     });
     MockApiClient.addMockResponse({
-      url: `/organizations/${organization.slug}/events/`,
+      url: `/organizations/${defaultInit.organization.slug}/events/`,
       statusCode: 200,
       body: {
         data: [
@@ -120,7 +120,7 @@ describe('groupDetails', () => {
       },
     });
     MockApiClient.addMockResponse({
-      url: `/organizations/${organization.slug}/environments/`,
+      url: `/organizations/${defaultInit.organization.slug}/environments/`,
       body: TestStubs.Environments(),
     });
     MockApiClient.addMockResponse({
@@ -142,7 +142,7 @@ describe('groupDetails', () => {
 
     expect(screen.queryByText(group.title)).not.toBeInTheDocument();
 
-    act(() => ProjectsStore.loadInitialData(organization.projects));
+    act(() => ProjectsStore.loadInitialData(defaultInit.organization.projects));
 
     expect(await screen.findByText(group.title, {exact: false})).toBeInTheDocument();
 
@@ -195,9 +195,16 @@ describe('groupDetails', () => {
   });
 
   it('fetches issue details for a given environment', async function () {
-    createWrapper({
-      selection: {environments: ['staging']},
+    const init = initializeOrg({
+      router: {
+        ...initRouter,
+        location: TestStubs.location({
+          ...initRouter.location,
+          query: {environment: 'staging'},
+        }),
+      },
     });
+    createWrapper({router: init.router});
 
     await waitFor(() =>
       expect(screen.queryByTestId('loading-indicator')).not.toBeInTheDocument()
@@ -259,7 +266,10 @@ describe('groupDetails', () => {
         substatus: 'ongoing',
       },
     });
-    createWrapper(undefined, {...organization, features: ['escalating-issues-ui']});
+    createWrapper({
+      ...defaultInit,
+      organization: {...defaultInit.organization, features: ['escalating-issues-ui']},
+    });
     expect(await screen.findByText('Ongoing')).toBeInTheDocument();
   });
 

static/app/views/issueDetails/groupDetails.tsx

@@ -9,8 +9,8 @@ import {
 import {browserHistory, RouteComponentProps} from 'react-router';
 import styled from '@emotion/styled';
 import * as Sentry from '@sentry/react';
+import isEmpty from 'lodash/isEmpty';
 
-import {fetchOrganizationEnvironments} from 'sentry/actionCreators/environments';
 import LoadingError from 'sentry/components/loadingError';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import PageFiltersContainer from 'sentry/components/organizations/pageFilters/container';
@@ -54,6 +54,7 @@ import {
   getGroupReprocessingStatus,
   markEventSeen,
   ReprocessingStatus,
+  useEnvironmentsFromUrl,
   useFetchIssueTagsForDetailsPage,
 } from './utils';
 
@@ -64,8 +65,6 @@ type RouteProps = RouteComponentProps<RouterParams, {}>;
 
 type GroupDetailsProps = {
   children: React.ReactNode;
-  environments: string[];
-  isGlobalSelectionReady: boolean;
   organization: Organization;
   projects: Project[];
 };
@@ -89,12 +88,9 @@ interface GroupDetailsContentProps extends GroupDetailsProps, FetchGroupDetailsS
   project: Project;
 }
 
-function getGroupQuery({
-  environments,
-}: Pick<GroupDetailsProps, 'environments'>): Record<string, string | string[]> {
-  // Note, we do not want to include the environment key at all if there are no environments
+function getGroupQuery({environments}: {environments: string[]}) {
   const query: Record<string, string | string[]> = {
-    ...(environments ? {environment: environments} : {}),
+    ...(!isEmpty(environments) ? {environment: environments} : {}),
     expand: ['inbox', 'owners'],
     collapse: ['release', 'tags'],
   };
@@ -102,6 +98,12 @@ function getGroupQuery({
   return query;
 }
 
+function getEventQuery({environments}: {environments: string[]}) {
+  const query = !isEmpty(environments) ? {environment: environments} : {};
+
+  return query;
+}
+
 function getFetchDataRequestErrorType(status?: number | null): Error {
   if (!status) {
     return null;
@@ -241,14 +243,8 @@ function useRefetchGroupForReprocessing({
 }
 
 function useFetchOnMount({fetchData}: Pick<FetchGroupDetailsState, 'fetchData'>) {
-  const api = useApi();
-  const organization = useOrganization();
-
   useEffect(() => {
     fetchData();
-
-    // Fetch environments early - used in GroupEventDetailsContainer
-    fetchOrganizationEnvironments(api, organization.slug);
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);
 }
@@ -273,13 +269,7 @@ function useEventApiQuery(
   return isLatest ? latestEventQuery : otherEventQuery;
 }
 
-function useFetchGroupDetails({
-  isGlobalSelectionReady,
-  environments,
-}: Pick<
-  GroupDetailsProps,
-  'isGlobalSelectionReady' | 'environments'
->): FetchGroupDetailsState {
+function useFetchGroupDetails(): FetchGroupDetailsState {
   const api = useApi();
   const organization = useOrganization();
   const router = useRouter();
@@ -295,6 +285,8 @@ function useFetchGroupDetails({
   const [errorType, setErrorType] = useState<Error | null>(null);
   const [event, setEvent] = useState<Event | null>(null);
 
+  const environments = useEnvironmentsFromUrl();
+
   const groupId = params.groupId;
   const eventId = params.eventId ?? 'latest';
 
@@ -310,7 +302,7 @@ function useFetchGroupDetails({
     isLoading: loadingEvent,
     isError,
     refetch: refetchEvent,
-  } = useEventApiQuery(eventId, [eventUrl, {query: eventQuery}]);
+  } = useEventApiQuery(eventId, [eventUrl, {query: getEventQuery({environments})}]);
 
   useEffect(() => {
     if (eventData) {
@@ -335,10 +327,6 @@ function useFetchGroupDetails({
 
   const fetchData = useCallback(async () => {
     // Need to wait for global selection store to be ready before making request
-    if (!isGlobalSelectionReady) {
-      return;
-    }
-
     try {
       const groupResponse = await api.requestPromise(`/issues/${params.groupId}/`, {
         query: getGroupQuery({environments}),
@@ -405,7 +393,6 @@ function useFetchGroupDetails({
     environments,
     fetchGroupReleases,
     handleError,
-    isGlobalSelectionReady,
     location,
     organization,
     params,
@@ -619,7 +606,6 @@ function GroupDetailsContentError({
 }
 
 function GroupDetailsContent({
-  environments,
   children,
   group,
   project,
@@ -634,6 +620,8 @@ function GroupDetailsContent({
   const {currentTab, baseUrl} = getCurrentRouteInfo({group, event, router, organization});
   const groupReprocessingStatus = getGroupReprocessingStatus(group);
 
+  const environments = useEnvironmentsFromUrl();
+
   useEffect(() => {
     if (
       currentTab === Tab.DETAILS &&
@@ -717,28 +705,25 @@ function GroupDetails(props: GroupDetailsProps) {
   const location = useLocation();
   const router = useRouter();
 
-  const {fetchData, project, group, ...fetchGroupDetailsProps} =
-    useFetchGroupDetails(props);
+  const {fetchData, project, group, ...fetchGroupDetailsProps} = useFetchGroupDetails();
 
   const previousPathname = usePrevious(location.pathname);
   const previousEventId = usePrevious(router.params.eventId);
-  const previousIsGlobalSelectionReady = usePrevious(props.isGlobalSelectionReady);
+
+  const environments = useEnvironmentsFromUrl();
 
   const {data} = useFetchIssueTagsForDetailsPage(
     {
       groupId: router.params.groupId,
-      environment: props.environments,
+      environment: environments,
     },
     // Don't want this query to take precedence over the main requests
-    {enabled: props.isGlobalSelectionReady && defined(group)}
+    {enabled: defined(group)}
   );
   const isSampleError = data?.some(tag => tag.key === 'sample_event') ?? false;
 
   useEffect(() => {
-    const globalSelectionReadyChanged =
-      previousIsGlobalSelectionReady !== props.isGlobalSelectionReady;
-
-    if (globalSelectionReadyChanged || location.pathname !== previousPathname) {
+    if (location.pathname !== previousPathname) {
       fetchData();
     }
   }, [
@@ -746,9 +731,7 @@ function GroupDetails(props: GroupDetailsProps) {
     group,
     location.pathname,
     previousEventId,
-    previousIsGlobalSelectionReady,
     previousPathname,
-    props.isGlobalSelectionReady,
     router.params.eventId,
   ]);
 

static/app/views/issueDetails/groupEventDetails/groupEventDetails.spec.tsx

@@ -12,12 +12,14 @@ import GroupEventDetails, {
   GroupEventDetailsProps,
 } from 'sentry/views/issueDetails/groupEventDetails/groupEventDetails';
 import {ReprocessingStatus} from 'sentry/views/issueDetails/utils';
+import {RouteContext} from 'sentry/views/routeContext';
 
 const TRACE_ID = '797cda4e24844bdc90e0efe741616047';
 
 const makeDefaultMockData = (
   organization?: Organization,
-  project?: Project
+  project?: Project,
+  environments?: string[]
 ): {
   event: Event;
   group: Group;
@@ -29,7 +31,13 @@ const makeDefaultMockData = (
     organization: organization ?? initializeOrg().organization,
     project: project ?? initializeOrg().project,
     group: TestStubs.Group(),
-    router: TestStubs.router({}),
+    router: TestStubs.router({
+      location: TestStubs.location({
+        query: {
+          environment: environments,
+        },
+      }),
+    }),
     event: TestStubs.Event({
       size: 1,
       dateCreated: '2019-03-20T00:00:00.000Z',
@@ -51,10 +59,13 @@ const makeDefaultMockData = (
   };
 };
 
-function TestComponent(props: Partial<GroupEventDetailsProps>) {
+function TestComponent(
+  props: Partial<GroupEventDetailsProps> & {environments?: string[]}
+) {
   const {organization, project, group, event, router} = makeDefaultMockData(
     props.organization,
-    props.project
+    props.project,
+    props.environments ?? ['dev']
   );
 
   const mergedProps: GroupEventDetailsProps = {
@@ -63,7 +74,6 @@ function TestComponent(props: Partial<GroupEventDetailsProps>) {
     event,
     project,
     organization,
-    environments: [{id: '1', name: 'dev', displayName: 'Dev'}],
     params: {groupId: group.id, eventId: '1'},
     router,
     location: {} as Location<any>,
@@ -78,7 +88,18 @@ function TestComponent(props: Partial<GroupEventDetailsProps>) {
     ...props,
   };
 
-  return <GroupEventDetails {...mergedProps} />;
+  return (
+    <RouteContext.Provider
+      value={{
+        router,
+        location: router.location,
+        params: router.params,
+        routes: router.routes,
+      }}
+    >
+      <GroupEventDetails {...mergedProps} />;
+    </RouteContext.Provider>
+  );
 }
 
 const mockedTrace = (project: Project) => {
@@ -263,9 +284,7 @@ describe('groupEventDetails', () => {
     });
     expect(browserHistory.replace).not.toHaveBeenCalled();
 
-    rerender(
-      <TestComponent environments={[{id: '1', name: 'prod', displayName: 'Prod'}]} />
-    );
+    rerender(<TestComponent environments={['prod']} />);
 
     await waitFor(() => expect(browserHistory.replace).toHaveBeenCalled());
   });

static/app/views/issueDetails/groupEventDetails/groupEventDetails.tsx

@@ -19,7 +19,6 @@ import ResolutionBox from 'sentry/components/resolutionBox';
 import {space} from 'sentry/styles/space';
 import {
   BaseGroupStatusReprocessing,
-  Environment,
   Group,
   GroupActivityReprocess,
   Organization,
@@ -42,6 +41,7 @@ import {
   getEventEnvironment,
   getGroupMostRecentActivity,
   ReprocessingStatus,
+  useEnvironmentsFromUrl,
 } from '../utils';
 
 const IssuePriorityFeedback = HookOrDefault({
@@ -51,7 +51,6 @@ const IssuePriorityFeedback = HookOrDefault({
 export interface GroupEventDetailsProps
   extends RouteComponentProps<{groupId: string; eventId?: string}, {}> {
   api: Client;
-  environments: Environment[];
   eventError: boolean;
   group: Group;
   groupReprocessingStatus: ReprocessingStatus;
@@ -67,7 +66,6 @@ function GroupEventDetails(props: GroupEventDetailsProps) {
     group,
     project,
     organization,
-    environments,
     location,
     event,
     groupReprocessingStatus,
@@ -85,6 +83,7 @@ function GroupEventDetails(props: GroupEventDetailsProps) {
   const mostRecentActivity = getGroupMostRecentActivity(activities);
   const orgSlug = organization.slug;
   const projectId = project.id;
+  const environments = useEnvironmentsFromUrl();
   const prevEnvironment = usePrevious(environments);
   const prevEvent = usePrevious(event);
 
@@ -116,7 +115,7 @@ function GroupEventDetails(props: GroupEventDetailsProps) {
     ) {
       const shouldRedirect =
         environments.length > 0 &&
-        !environments.find(env => env.name === getEventEnvironment(prevEvent as Event));
+        !environments.find(env => env === getEventEnvironment(prevEvent as Event));
 
       if (shouldRedirect) {
         browserHistory.replace(