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

This reverts commit c4e28fc5bed384a0230e3d3d35838afe596c39fc.

Co-authored-by: malwilley <10888943+malwilley@users.noreply.github.com>

static/app/actionCreators/environments.tsx

@@ -0,0 +1,28 @@
+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,9 +2,10 @@ 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: string[];
+  environments: Environment[];
   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, Group, Organization, Project} from 'sentry/types';
+import {CurrentRelease, Environment, Group, Organization, Project} from 'sentry/types';
 import getDynamicText from 'sentry/utils/getDynamicText';
 
 type Props = {
   allEnvironments: Group | undefined;
   currentRelease: CurrentRelease | undefined;
-  environments: string[];
+  environments: Environment[];
   group: Group | undefined;
   organization: Organization;
   project: Project;
@@ -30,14 +30,17 @@ function GroupReleaseStats({
   group,
   currentRelease,
 }: Props) {
-  const environment = environments.length > 0 ? environments.join(', ') : undefined;
+  const environment =
+    environments.length > 0
+      ? environments.map(env => env.displayName).join(', ')
+      : undefined;
   const environmentLabel = environment ? environment : t('All Environments');
 
   const shortEnvironmentLabel =
     environments.length > 1
       ? t('selected environments')
       : environments.length === 1
-      ? environments[0]
+      ? environments[0].displayName
       : 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 {Event, Organization, Project} from 'sentry/types';
+import {Environment, 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: string[];
+  environments: Environment[];
   groupId: string;
   project: Project;
   tagKeys: string[];
@@ -94,7 +94,7 @@ export default function TagFacets({
 
   const {isLoading, isError, data, refetch} = useFetchIssueTagsForDetailsPage({
     groupId,
-    environment: environments,
+    environment: environments.map(({name}) => name),
   });
 
   const tagsData = useMemo(() => {

static/app/stores/organizationEnvironmentsStore.spec.jsx

@@ -0,0 +1,52 @@
+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

@@ -0,0 +1,73 @@
+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,6 +19,7 @@ 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},
@@ -36,22 +37,20 @@ describe('groupDetails', () => {
     },
   ];
 
-  const initRouter = {
-    location: {
-      pathname: `/organizations/org-slug/issues/${group.id}/`,
-      query: {},
-      search: '?foo=bar',
-      hash: '#hash',
-    },
-    params: {
-      groupId: group.id,
-    },
-    routes,
-  };
-
-  const defaultInit = initializeOrg({
+  const {organization, router, routerContext} = initializeOrg({
     project,
-    router: initRouter,
+    router: {
+      location: {
+        pathname: `/organizations/org-slug/issues/${group.id}/`,
+        query: {},
+        search: '?foo=bar',
+        hash: '#hash',
+      },
+      params: {
+        groupId: group.id,
+      },
+      routes,
+    },
   });
 
   function MockComponent({group: groupProp, environments, eventError}) {
@@ -65,22 +64,23 @@ describe('groupDetails', () => {
     );
   }
 
-  const createWrapper = (init = defaultInit) => {
+  const createWrapper = (props = {selection}, org = organization) => {
     return render(
       <GroupDetails
-        {...init.router}
-        router={init.router}
-        organization={init.organization}
+        {...router}
+        router={router}
+        selection={props.selection}
+        organization={org}
       >
         <MockComponent />
       </GroupDetails>,
-      {context: init.routerContext, organization: init.organization, router: init.router}
+      {context: routerContext, organization: org, router}
     );
   };
 
   beforeEach(() => {
-    OrganizationStore.onUpdate(defaultInit.organization);
-    act(() => ProjectsStore.loadInitialData(defaultInit.organization.projects));
+    OrganizationStore.onUpdate(organization);
+    act(() => ProjectsStore.loadInitialData(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/${defaultInit.organization.slug}/events/`,
+      url: `/organizations/${organization.slug}/events/`,
       statusCode: 200,
       body: {
         data: [
@@ -120,7 +120,7 @@ describe('groupDetails', () => {
       },
     });
     MockApiClient.addMockResponse({
-      url: `/organizations/${defaultInit.organization.slug}/environments/`,
+      url: `/organizations/${organization.slug}/environments/`,
       body: TestStubs.Environments(),
     });
     MockApiClient.addMockResponse({
@@ -142,7 +142,7 @@ describe('groupDetails', () => {
 
     expect(screen.queryByText(group.title)).not.toBeInTheDocument();
 
-    act(() => ProjectsStore.loadInitialData(defaultInit.organization.projects));
+    act(() => ProjectsStore.loadInitialData(organization.projects));
 
     expect(await screen.findByText(group.title, {exact: false})).toBeInTheDocument();
 
@@ -195,16 +195,9 @@ describe('groupDetails', () => {
   });
 
   it('fetches issue details for a given environment', async function () {
-    const init = initializeOrg({
-      router: {
-        ...initRouter,
-        location: TestStubs.location({
-          ...initRouter.location,
-          query: {environment: 'staging'},
-        }),
-      },
+    createWrapper({
+      selection: {environments: ['staging']},
     });
-    createWrapper({router: init.router});
 
     await waitFor(() =>
       expect(screen.queryByTestId('loading-indicator')).not.toBeInTheDocument()
@@ -266,10 +259,7 @@ describe('groupDetails', () => {
         substatus: 'ongoing',
       },
     });
-    createWrapper({
-      ...defaultInit,
-      organization: {...defaultInit.organization, features: ['escalating-issues-ui']},
-    });
+    createWrapper(undefined, {...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,7 +54,6 @@ import {
   getGroupReprocessingStatus,
   markEventSeen,
   ReprocessingStatus,
-  useEnvironmentsFromUrl,
   useFetchIssueTagsForDetailsPage,
 } from './utils';
 
@@ -65,6 +64,8 @@ type RouteProps = RouteComponentProps<RouterParams, {}>;
 
 type GroupDetailsProps = {
   children: React.ReactNode;
+  environments: string[];
+  isGlobalSelectionReady: boolean;
   organization: Organization;
   projects: Project[];
 };
@@ -88,9 +89,12 @@ interface GroupDetailsContentProps extends GroupDetailsProps, FetchGroupDetailsS
   project: Project;
 }
 
-function getGroupQuery({environments}: {environments: string[]}) {
+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
   const query: Record<string, string | string[]> = {
-    ...(!isEmpty(environments) ? {environment: environments} : {}),
+    ...(environments ? {environment: environments} : {}),
     expand: ['inbox', 'owners'],
     collapse: ['release', 'tags'],
   };
@@ -98,12 +102,6 @@ function getGroupQuery({environments}: {environments: string[]}) {
   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;
@@ -243,8 +241,14 @@ 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
   }, []);
 }
@@ -269,7 +273,13 @@ function useEventApiQuery(
   return isLatest ? latestEventQuery : otherEventQuery;
 }
 
-function useFetchGroupDetails(): FetchGroupDetailsState {
+function useFetchGroupDetails({
+  isGlobalSelectionReady,
+  environments,
+}: Pick<
+  GroupDetailsProps,
+  'isGlobalSelectionReady' | 'environments'
+>): FetchGroupDetailsState {
   const api = useApi();
   const organization = useOrganization();
   const router = useRouter();
@@ -285,8 +295,6 @@ function useFetchGroupDetails(): FetchGroupDetailsState {
   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';
 
@@ -302,7 +310,7 @@ function useFetchGroupDetails(): FetchGroupDetailsState {
     isLoading: loadingEvent,
     isError,
     refetch: refetchEvent,
-  } = useEventApiQuery(eventId, [eventUrl, {query: getEventQuery({environments})}]);
+  } = useEventApiQuery(eventId, [eventUrl, {query: eventQuery}]);
 
   useEffect(() => {
     if (eventData) {
@@ -327,6 +335,10 @@ function useFetchGroupDetails(): FetchGroupDetailsState {
 
   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}),
@@ -393,6 +405,7 @@ function useFetchGroupDetails(): FetchGroupDetailsState {
     environments,
     fetchGroupReleases,
     handleError,
+    isGlobalSelectionReady,
     location,
     organization,
     params,
@@ -606,6 +619,7 @@ function GroupDetailsContentError({
 }
 
 function GroupDetailsContent({
+  environments,
   children,
   group,
   project,
@@ -620,8 +634,6 @@ function GroupDetailsContent({
   const {currentTab, baseUrl} = getCurrentRouteInfo({group, event, router, organization});
   const groupReprocessingStatus = getGroupReprocessingStatus(group);
 
-  const environments = useEnvironmentsFromUrl();
-
   useEffect(() => {
     if (
       currentTab === Tab.DETAILS &&
@@ -705,25 +717,28 @@ function GroupDetails(props: GroupDetailsProps) {
   const location = useLocation();
   const router = useRouter();
 
-  const {fetchData, project, group, ...fetchGroupDetailsProps} = useFetchGroupDetails();
+  const {fetchData, project, group, ...fetchGroupDetailsProps} =
+    useFetchGroupDetails(props);
 
   const previousPathname = usePrevious(location.pathname);
   const previousEventId = usePrevious(router.params.eventId);
-
-  const environments = useEnvironmentsFromUrl();
+  const previousIsGlobalSelectionReady = usePrevious(props.isGlobalSelectionReady);
 
   const {data} = useFetchIssueTagsForDetailsPage(
     {
       groupId: router.params.groupId,
-      environment: environments,
+      environment: props.environments,
     },
     // Don't want this query to take precedence over the main requests
-    {enabled: defined(group)}
+    {enabled: props.isGlobalSelectionReady && defined(group)}
   );
   const isSampleError = data?.some(tag => tag.key === 'sample_event') ?? false;
 
   useEffect(() => {
-    if (location.pathname !== previousPathname) {
+    const globalSelectionReadyChanged =
+      previousIsGlobalSelectionReady !== props.isGlobalSelectionReady;
+
+    if (globalSelectionReadyChanged || location.pathname !== previousPathname) {
       fetchData();
     }
   }, [
@@ -731,7 +746,9 @@ function GroupDetails(props: GroupDetailsProps) {
     group,
     location.pathname,
     previousEventId,
+    previousIsGlobalSelectionReady,
     previousPathname,
+    props.isGlobalSelectionReady,
     router.params.eventId,
   ]);
 

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

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

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

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