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

JonasBa · web-flow · commit d48a00858c0b · 2023-11-27T17:48:28.000Z
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~ &lt;- should never happen as some places load
stats together with the group request
diff --git a/static/app/components/eventOrGroupExtraDetails.tsx b/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 =
diff --git a/static/app/components/stream/group.tsx b/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>
diff --git a/static/app/components/stream/groupChart.tsx b/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[],
diff --git a/static/app/stores/groupStore.spec.tsx b/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 = [];
diff --git a/static/app/stores/groupStore.tsx b/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);
diff --git a/static/app/utils/parseApiError.tsx b/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
diff --git a/static/app/views/issueList/groupStatsProvider.tsx b/static/app/views/issueList/groupStatsProvider.tsx
diff --git a/static/app/views/issueList/overview.tsx b/static/app/views/issueList/overview.tsx
