ref(groupstats): decouple groupStats from GroupStore (#60259)

Decoupling stats from groups and GroupStore allows us to optimize the
react tree rerender and avoid large blocking updates (as we no longer
create a fresh group reference for each group). We also move the stats
state to use react query, which means we can leverage caching on
pagination and navigation loading.

~I will open a followup pr to remove stats from the group type and
finalize the decoupling~ <- should never happen as some places load
stats together with the group request

static/app/components/eventOrGroupExtraDetails.tsx

@@ -18,6 +18,7 @@ import type {Group, Organization} from 'sentry/types';
 import {Event} from 'sentry/types/event';
 import {projectCanLinkToReplay} from 'sentry/utils/replays/projectSupportsReplay';
 import withOrganization from 'sentry/utils/withOrganization';
+import {useGroupStats} from 'sentry/views/issueList/groupStatsProvider';
 
 type Props = {
   data: Event | Group;
@@ -43,13 +44,13 @@ function EventOrGroupExtraDetails({
     annotations,
     shortId,
     project,
-    lifetime,
-    isUnhandled,
     inbox,
     status,
     substatus,
   } = data as Group;
 
+  const {lifetime, isUnhandled} = useGroupStats(data as Group);
+
   const issuesPath = `/organizations/${organization.slug}/issues/`;
 
   const showReplayCount =

static/app/components/stream/group.tsx

@@ -43,6 +43,7 @@ import {getConfigForIssueType} from 'sentry/utils/issueTypeConfig';
 import usePageFilters from 'sentry/utils/usePageFilters';
 import withOrganization from 'sentry/utils/withOrganization';
 import {TimePeriodType} from 'sentry/views/alerts/rules/metric/details/constants';
+import {useGroupStats} from 'sentry/views/issueList/groupStatsProvider';
 import {
   DISCOVER_EXCLUSION_FIELDS,
   getTabs,
@@ -246,7 +247,8 @@ function BaseGroupRow({
   };
 
   const renderReprocessingColumns = () => {
-    const {statusDetails, count} = group as GroupReprocessing;
+    const {count} = stats;
+    const {statusDetails} = group as GroupReprocessing;
     const {info, pendingEvents} = statusDetails;
 
     if (!info) {
@@ -284,18 +286,20 @@ function BaseGroupRow({
     );
   };
 
+  const stats = useGroupStats(group);
+
   // Use data.filtered to decide on which value to use
   // In case of the query has filters but we avoid showing both sets of filtered/unfiltered stats
   // we use useFilteredStats param passed to Group for deciding
-  const primaryCount = group.filtered ? group.filtered.count : group.count;
-  const secondaryCount = group.filtered ? group.count : undefined;
-  const primaryUserCount = group.filtered ? group.filtered.userCount : group.userCount;
-  const secondaryUserCount = group.filtered ? group.userCount : undefined;
+  const primaryCount = stats.filtered ? stats.filtered.count : stats.count;
+  const secondaryCount = stats.filtered ? stats.count : undefined;
+  const primaryUserCount = stats.filtered ? stats.filtered.userCount : stats.userCount;
+  const secondaryUserCount = stats.filtered ? stats.userCount : undefined;
   // preview stats
-  const lastTriggeredDate = group.lastTriggered;
+  const lastTriggeredDate = stats.lastTriggered;
 
   const showSecondaryPoints = Boolean(
-    withChart && group && group.filtered && statsPeriod && useFilteredStats
+    withChart && group && stats.filtered && statsPeriod && useFilteredStats
   );
 
   const groupCategoryCountTitles: Record<IssueCategory, string> = {
@@ -315,24 +319,24 @@ function BaseGroupRow({
         title={
           <CountTooltipContent>
             <h4>{groupCategoryCountTitles[group.issueCategory]}</h4>
-            {group.filtered && (
+            {stats.filtered && (
               <Fragment>
                 <div>{queryFilterDescription ?? t('Matching filters')}</div>
                 <Link to={getDiscoverUrl(true)}>
-                  <Count value={group.filtered?.count} />
+                  <Count value={stats.filtered?.count} />
                 </Link>
               </Fragment>
             )}
             <Fragment>
               <div>{t('Total in %s', summary)}</div>
               <Link to={getDiscoverUrl()}>
-                <Count value={group.count} />
+                <Count value={stats.count} />
               </Link>
             </Fragment>
-            {group.lifetime && (
+            {stats.lifetime && (
               <Fragment>
                 <div>{t('Since issue began')}</div>
-                <Count value={group.lifetime.count} />
+                <Count value={stats.lifetime.count} />
               </Fragment>
             )}
           </CountTooltipContent>
@@ -355,24 +359,24 @@ function BaseGroupRow({
       title={
         <CountTooltipContent>
           <h4>{t('Affected Users')}</h4>
-          {group.filtered && (
+          {stats.filtered && (
             <Fragment>
               <div>{queryFilterDescription ?? t('Matching filters')}</div>
               <Link to={getDiscoverUrl(true)}>
-                <Count value={group.filtered?.userCount} />
+                <Count value={stats.filtered?.userCount} />
               </Link>
             </Fragment>
           )}
           <Fragment>
             <div>{t('Total in %s', summary)}</div>
             <Link to={getDiscoverUrl()}>
-              <Count value={group.userCount} />
+              <Count value={stats.userCount} />
             </Link>
           </Fragment>
-          {group.lifetime && (
+          {stats.lifetime && (
             <Fragment>
               <div>{t('Since issue began')}</div>
-              <Count value={group.lifetime.userCount} />
+              <Count value={stats.lifetime.userCount} />
             </Fragment>
           )}
         </CountTooltipContent>

static/app/components/stream/groupChart.tsx

@@ -8,6 +8,7 @@ import {Group, TimeseriesValue} from 'sentry/types';
 import {Series} from 'sentry/types/echarts';
 import {formatAbbreviatedNumber} from 'sentry/utils/formatters';
 import theme from 'sentry/utils/theme';
+import {useGroupStats} from 'sentry/views/issueList/groupStatsProvider';
 
 function asChartPoint(point: [number, number]): {name: number | string; value: number} {
   return {
@@ -33,14 +34,15 @@ function GroupChart({
   height = 24,
   showMarkLine = false,
 }: Props) {
+  const groupStats = useGroupStats(data);
   const stats: ReadonlyArray<TimeseriesValue> = statsPeriod
-    ? data.filtered
-      ? data.filtered.stats?.[statsPeriod]
-      : data.stats?.[statsPeriod]
+    ? groupStats.filtered
+      ? groupStats.filtered.stats?.[statsPeriod]
+      : groupStats.stats?.[statsPeriod]
     : EMPTY_STATS;
 
   const secondaryStats: TimeseriesValue[] | null =
-    statsPeriod && data.filtered ? data.stats[statsPeriod] : null;
+    statsPeriod && groupStats.filtered ? groupStats.stats[statsPeriod] : null;
 
   const [series, colors, emphasisColors]: [
     Series[],

static/app/stores/groupStore.spec.tsx

@@ -1,7 +1,7 @@
 import {Project} from 'sentry-fixture/project';
 
 import GroupStore from 'sentry/stores/groupStore';
-import {Group, GroupActivityType, GroupStats, TimeseriesValue} from 'sentry/types';
+import {Group, GroupActivityType} from 'sentry/types';
 
 const MOCK_PROJECT = TestStubs.Project();
 
@@ -105,48 +105,6 @@ describe('GroupStore', function () {
     });
   });
 
-  describe('onPopulateStats()', function () {
-    const stats: Record<string, TimeseriesValue[]> = {auto: [[1611576000, 10]]};
-
-    beforeEach(function () {
-      jest.spyOn(GroupStore, 'trigger');
-      GroupStore.items = [g('1'), g('2'), g('3')];
-    });
-    afterEach(() => {
-      jest.restoreAllMocks();
-    });
-
-    it('should merge stats into existing groups', function () {
-      GroupStore.onPopulateStats(
-        ['1', '2', '3'],
-        [
-          {id: '1', stats} as GroupStats,
-          {id: '2', stats} as GroupStats,
-          {id: '3', stats} as GroupStats,
-        ]
-      );
-
-      const group = GroupStore.getAllItems()[0] as Group;
-
-      expect(group.stats).toEqual(stats);
-      expect(GroupStore.trigger).toHaveBeenCalledWith(new Set(['1', '2', '3']));
-    });
-
-    it('should not change current item ids', function () {
-      GroupStore.onPopulateStats(
-        ['2', '3'],
-        [{id: '2', stats} as GroupStats, {id: '3', stats} as GroupStats]
-      );
-
-      const group1 = GroupStore.getAllItems()[0] as Group;
-      const group2 = GroupStore.getAllItems()[1] as Group;
-
-      expect(GroupStore.trigger).toHaveBeenCalledWith(new Set(['2', '3']));
-      expect(group1.stats).not.toEqual(stats);
-      expect(group2.stats).toEqual(stats);
-    });
-  });
-
   describe('getAllItems()', function () {
     it('Merges pending changes into items', function () {
       GroupStore.items = [];

static/app/stores/groupStore.tsx

@@ -3,7 +3,7 @@ import {createStore} from 'reflux';
 import {Indicator} from 'sentry/actionCreators/indicator';
 import {t} from 'sentry/locale';
 import IndicatorStore from 'sentry/stores/indicatorStore';
-import {Activity, BaseGroup, Group, GroupStats} from 'sentry/types';
+import {Activity, BaseGroup, Group} from 'sentry/types';
 import RequestError from 'sentry/utils/requestError/requestError';
 import toArray from 'sentry/utils/toArray';
 
@@ -70,8 +70,6 @@ interface GroupStoreDefinition extends CommonStoreDefinition<Item[]>, InternalDe
   onMergeError: (changeId: string, itemIds: ItemIds, response: any) => void;
   onMergeSuccess: (changeId: string, itemIds: ItemIds, response: any) => void;
 
-  onPopulateStats: (itemIds: ItemIds, response: GroupStats[]) => void;
-
   onUpdate: (changeId: string, itemIds: ItemIds, data: any) => void;
   onUpdateError: (changeId: string, itemIds: ItemIds, silent: boolean) => void;
   onUpdateSuccess: (changeId: string, itemIds: ItemIds, response: Partial<Group>) => void;
@@ -459,24 +457,6 @@ const storeConfig: GroupStoreDefinition = {
     this.pendingChanges.delete(changeId);
     this.updateItems(ids);
   },
-
-  onPopulateStats(itemIds, response) {
-    // Organize stats by id
-    const groupStatsMap = response.reduce<Record<string, GroupStats>>(
-      (map, stats) => ({...map, [stats.id]: stats}),
-      {}
-    );
-
-    this.items.forEach((item, idx) => {
-      if (itemIds?.includes(item.id)) {
-        this.items[idx] = {
-          ...item,
-          ...groupStatsMap[item.id],
-        };
-      }
-    });
-    this.updateItems(itemIds);
-  },
 };
 
 const GroupStore = createStore(storeConfig);

static/app/utils/parseApiError.tsx

@@ -1,6 +1,7 @@
 import {ResponseMeta} from 'sentry/api';
+import RequestError from 'sentry/utils/requestError/requestError';
 
-export default function parseApiError(resp: ResponseMeta): string {
+export default function parseApiError(resp: ResponseMeta | RequestError): string {
   const {detail} = (resp && resp.responseJSON) || ({} as object);
 
   // return immediately if string

static/app/views/issueList/groupStatsProvider.tsx

@@ -0,0 +1,134 @@
+import {createContext, useContext, useEffect} from 'react';
+import {dropUndefinedKeys} from '@sentry/utils';
+import * as reactQuery from '@tanstack/react-query';
+
+import {ApiResult} from 'sentry/api';
+import type {Group, GroupStats, Organization, PageFilters} from 'sentry/types';
+import {getUtcDateString} from 'sentry/utils/dates';
+import {UseQueryResult} from 'sentry/utils/queryClient';
+import RequestError from 'sentry/utils/requestError/requestError';
+import useApi from 'sentry/utils/useApi';
+
+function getEndpointParams(
+  p: Pick<GroupStatsProviderProps, 'selection' | 'period' | 'query' | 'groupIds'>
+): StatEndpointParams {
+  const params: StatEndpointParams = {
+    project: p.selection.projects,
+    environment: p.selection.environments,
+    groupStatsPeriod: p.period,
+    query: p.query,
+    groups: p.groupIds,
+    ...p.selection.datetime,
+  };
+
+  if (p.selection.datetime.period) {
+    delete params.period;
+    params.statsPeriod = p.selection.datetime.period;
+  }
+  if (params.end) {
+    params.end = getUtcDateString(params.end);
+  }
+  if (params.start) {
+    params.start = getUtcDateString(params.start);
+  }
+
+  return dropUndefinedKeys(params);
+}
+
+const GroupStatsContext = createContext<UseQueryResult<
+  Record<string, GroupStats>
+> | null>(null);
+
+export function useGroupStats(group: Group): GroupStats {
+  const ctx = useContext(GroupStatsContext);
+
+  if (!ctx) {
+    return group;
+  }
+
+  return ctx.data?.[group.id] ?? group;
+}
+
+interface StatEndpointParams extends Partial<PageFilters['datetime']> {
+  environment: string[];
+  groups: Group['id'][];
+  project: number[];
+  cursor?: string;
+  expand?: string | string[];
+  groupStatsPeriod?: string | null;
+  page?: number | string;
+  query?: string | undefined;
+  sort?: string;
+  statsPeriod?: string | null;
+}
+
+export type GroupStatsQuery = UseQueryResult<Record<string, GroupStats>, RequestError>;
+
+export interface GroupStatsProviderProps {
+  children: React.ReactNode;
+  groupIds: Group['id'][];
+  organization: Organization;
+  period: string;
+
+  selection: PageFilters;
+  onStatsQuery?: (query: GroupStatsQuery) => void;
+  query?: string;
+}
+
+export function GroupStatsProvider(props: GroupStatsProviderProps) {
+  const api = useApi();
+
+  const queryFn = (): Promise<Record<string, GroupStats>> => {
+    const promise = api
+      .requestPromise<true>(`/organizations/${props.organization.slug}/issues-stats/`, {
+        method: 'GET',
+        query: getEndpointParams({
+          selection: props.selection,
+          period: props.period,
+          query: props.query,
+          groupIds: props.groupIds,
+        }),
+        includeAllArgs: true,
+      })
+      .then((resp: ApiResult<GroupStats[]>): Record<string, GroupStats> => {
+        const map: Record<string, GroupStats> = {};
+        if (!resp || !Array.isArray(resp[0])) {
+          return map;
+        }
+        for (const stat of resp[0]) {
+          map[stat.id] = stat;
+        }
+        return map;
+      });
+
+    return promise;
+  };
+
+  const statsQuery = reactQuery.useQuery<Record<string, GroupStats>, RequestError>(
+    [
+      `/organizations/${props.organization.slug}/issues-stats/`,
+      props.selection,
+      props.period,
+      props.query,
+      props.groupIds,
+    ],
+    queryFn,
+    {
+      enabled: props.groupIds.length > 0,
+      staleTime: Infinity,
+    }
+  );
+
+  const onStatsQuery = props.onStatsQuery;
+  useEffect(() => {
+    onStatsQuery?.(statsQuery);
+    // We only want to fire the observer when the status changes
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [statsQuery.status, onStatsQuery]);
+
+  return (
+    <GroupStatsContext.Provider value={statsQuery}>
+      {props.children}
+    </GroupStatsContext.Provider>
+  );
+}

static/app/views/issueList/overview.tsx

@@ -57,6 +57,10 @@ import withIssueTags from 'sentry/utils/withIssueTags';
 import withOrganization from 'sentry/utils/withOrganization';
 import withPageFilters from 'sentry/utils/withPageFilters';
 import withSavedSearches from 'sentry/utils/withSavedSearches';
+import {
+  GroupStatsProvider,
+  GroupStatsQuery,
+} from 'sentry/views/issueList/groupStatsProvider';
 import SavedIssueSearches from 'sentry/views/issueList/savedIssueSearches';
 
 import IssueListActions from './actions';
@@ -140,11 +144,6 @@ type CountsEndpointParams = Omit<EndpointParams, 'cursor' | 'page' | 'query'> &
   query: string[];
 };
 
-type StatEndpointParams = Omit<EndpointParams, 'cursor' | 'page'> & {
-  groups: string[];
-  expand?: string | string[];
-};
-
 class IssueListOverview extends Component<Props, State> {
   state: State = this.getInitialState();
 
@@ -302,7 +301,6 @@ class IssueListOverview extends Component<Props, State> {
 
   private _poller: any;
   private _lastRequest: any;
-  private _lastStatsRequest: any;
   private _lastFetchCountsRequest: any;
 
   getQueryFromSavedSearchOrLocation({
@@ -458,48 +456,6 @@ class IssueListOverview extends Component<Props, State> {
     loadOrganizationTags(api, organization.slug, selection);
   }
 
-  fetchStats = (groups: string[]) => {
-    // If we have no groups to fetch, just skip stats
-    if (!groups.length) {
-      return;
-    }
-    const requestParams: StatEndpointParams = {
-      ...this.getEndpointParams(),
-      groups,
-    };
-    // If no stats period values are set, use default
-    if (!requestParams.statsPeriod && !requestParams.start) {
-      requestParams.statsPeriod = DEFAULT_STATS_PERIOD;
-    }
-
-    this._lastStatsRequest = this.props.api.request(this.groupStatsEndpoint, {
-      method: 'GET',
-      data: qs.stringify(requestParams),
-      success: data => {
-        if (!data) {
-          return;
-        }
-        GroupStore.onPopulateStats(groups, data);
-        this.trackTabViewed(groups, data);
-      },
-      error: err => {
-        this.setState({
-          error: parseApiError(err),
-        });
-      },
-      complete: () => {
-        this._lastStatsRequest = null;
-
-        // End navigation transaction to prevent additional page requests from impacting page metrics.
-        // Other transactions include stacktrace preview request
-        const currentTransaction = Sentry.getCurrentHub().getScope()?.getTransaction();
-        if (currentTransaction?.op === 'navigation') {
-          currentTransaction.finish();
-        }
-      },
-    });
-  };
-
   fetchCounts = (currentQueryCount: number, fetchAllCounts: boolean) => {
     const {organization} = this.props;
     const {queryCounts: _queryCounts} = this.state;
@@ -648,9 +604,6 @@ class IssueListOverview extends Component<Props, State> {
     if (this._lastRequest) {
       this._lastRequest.cancel();
     }
-    if (this._lastStatsRequest) {
-      this._lastStatsRequest.cancel();
-    }
     if (this._lastFetchCountsRequest) {
       this._lastFetchCountsRequest.cancel();
     }
@@ -693,8 +646,6 @@ class IssueListOverview extends Component<Props, State> {
         }
         GroupStore.add(data);
 
-        this.fetchStats(data.map((group: BaseGroup) => group.id));
-
         const hits = resp.getResponseHeader('X-Hits');
         const queryCount =
           typeof hits !== 'undefined' && hits ? parseInt(hits, 10) || 0 : 0;
@@ -1055,9 +1006,6 @@ class IssueListOverview extends Component<Props, State> {
     if (this._lastRequest) {
       this._lastRequest.cancel();
     }
-    if (this._lastStatsRequest) {
-      this._lastStatsRequest.cancel();
-    }
     if (this._lastFetchCountsRequest) {
       this._lastFetchCountsRequest.cancel();
     }
@@ -1215,6 +1163,32 @@ class IssueListOverview extends Component<Props, State> {
     };
   };
 
+  onStatsQuery = (query: GroupStatsQuery) => {
+    switch (query.status) {
+      case 'loading':
+        break;
+      case 'success':
+        const data = Object.values(query.data ?? {});
+        if (data && data[0]) {
+          // The type being cast was wrong in the previous implementations as well
+          // because inference was broken. Ignore it for now.
+          this.trackTabViewed(this.state.groupIds, data as Group[]);
+        }
+        const currentTransaction = Sentry.getCurrentHub().getScope()?.getTransaction();
+        if (currentTransaction?.op === 'navigation') {
+          currentTransaction.finish();
+        }
+        break;
+      case 'error':
+        this.setState({
+          // Missing our custom getResponseHeader function, but parseApiError does not require it
+          error: parseApiError(query?.error),
+        });
+        break;
+      default:
+    }
+  };
+
   render() {
     if (
       this.props.savedSearchLoading &&
@@ -1289,22 +1263,31 @@ class IssueListOverview extends Component<Props, State> {
                   showProject
                 />
                 <VisuallyCompleteWithData
-                  hasData={this.state.groupIds.length > 0}
                   id="IssueList-Body"
+                  hasData={this.state.groupIds.length > 0}
                   isLoading={this.state.issuesLoading}
                 >
-                  <GroupListBody
-                    memberList={this.state.memberList}
-                    groupStatsPeriod={this.getGroupStatsPeriod()}
+                  <GroupStatsProvider
                     groupIds={groupIds}
-                    displayReprocessingLayout={displayReprocessingActions}
-                    query={query}
-                    sort={this.getSort()}
-                    selectedProjectIds={selection.projects}
-                    loading={issuesLoading}
-                    error={error}
-                    refetchGroups={this.fetchData}
-                  />
+                    selection={this.props.selection}
+                    organization={this.props.organization}
+                    period={this.getGroupStatsPeriod()}
+                    query={this.getQuery()}
+                    onStatsQuery={this.onStatsQuery}
+                  >
+                    <GroupListBody
+                      memberList={this.state.memberList}
+                      groupStatsPeriod={this.getGroupStatsPeriod()}
+                      groupIds={groupIds}
+                      displayReprocessingLayout={displayReprocessingActions}
+                      sort={this.getSort()}
+                      selectedProjectIds={selection.projects}
+                      loading={issuesLoading}
+                      error={error}
+                      query={query}
+                      refetchGroups={this.fetchData}
+                    />
+                  </GroupStatsProvider>
                 </VisuallyCompleteWithData>
               </PanelBody>
             </Panel>