Browse Source

ref(tsc): Convert `ProjectApdexScoreCard` to FC (#78294)

Part of https://github.com/getsentry/frontend-tsc/issues/2
George Gritsouk 5 months ago
parent
commit
dd905774b0

+ 72 - 11
static/app/views/projectDetail/projectScoreCards/projectApdexScoreCard.spec.tsx

@@ -1,11 +1,12 @@
 import {OrganizationFixture} from 'sentry-fixture/organization';
 
-import {render} from 'sentry-test/reactTestingLibrary';
+import {render, screen} from 'sentry-test/reactTestingLibrary';
 
 import ProjectApdexScoreCard from 'sentry/views/projectDetail/projectScoreCards/projectApdexScoreCard';
 
 describe('ProjectDetail > ProjectApdex', function () {
-  let endpointMock: jest.Mock;
+  let currentDataEndpointMock: jest.Mock;
+  let previousDataEndpointMock: jest.Mock;
   const organization = OrganizationFixture();
 
   const selection = {
@@ -19,21 +20,36 @@ describe('ProjectDetail > ProjectApdex', function () {
     },
   };
 
-  beforeEach(function () {
-    endpointMock = MockApiClient.addMockResponse({
+  afterEach(function () {
+    MockApiClient.clearMockResponses();
+  });
+
+  it('renders apdex', async function () {
+    previousDataEndpointMock = MockApiClient.addMockResponse({
       url: `/organizations/${organization.slug}/events/`,
       body: {
-        data: [],
+        data: [
+          {
+            'apdex()': 0.678,
+          },
+        ],
       },
       status: 200,
     });
-  });
 
-  afterEach(function () {
-    MockApiClient.clearMockResponses();
-  });
+    currentDataEndpointMock = MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/events/`,
+      body: {
+        data: [
+          {
+            'apdex()': 0.781,
+          },
+        ],
+      },
+      status: 200,
+      match: [MockApiClient.matchQuery({statsPeriod: '14d'})],
+    });
 
-  it('calls api with apdex', function () {
     render(
       <ProjectApdexScoreCard
         organization={{...organization, features: ['discover-basic', 'performance-view']}}
@@ -43,7 +59,11 @@ describe('ProjectDetail > ProjectApdex', function () {
       />
     );
 
-    expect(endpointMock).toHaveBeenNthCalledWith(
+    expect(await screen.findByText('Apdex')).toBeInTheDocument();
+    expect(await screen.findByText('0.781')).toBeInTheDocument();
+    expect(await screen.findByText('0.103')).toBeInTheDocument();
+
+    expect(currentDataEndpointMock).toHaveBeenNthCalledWith(
       1,
       `/organizations/${organization.slug}/events/`,
       expect.objectContaining({
@@ -56,5 +76,46 @@ describe('ProjectDetail > ProjectApdex', function () {
         },
       })
     );
+
+    expect(previousDataEndpointMock).toHaveBeenNthCalledWith(
+      1,
+      `/organizations/${organization.slug}/events/`,
+      expect.objectContaining({
+        query: {
+          environment: [],
+          field: ['apdex()'],
+          project: ['1'],
+          query: 'event.type:transaction count():>0',
+          start: '2017-09-19T02:41:20',
+          end: '2017-10-03T02:41:20',
+        },
+      })
+    );
+  });
+
+  it('renders without performance', async function () {
+    const endpointMock = MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/events/`,
+      body: {
+        detail: 'test error',
+      },
+      status: 404,
+    });
+
+    render(
+      <ProjectApdexScoreCard
+        organization={{...organization, features: ['performance-view']}}
+        hasTransactions={false}
+        selection={selection}
+        isProjectStabilized
+        query="test-query"
+      />
+    );
+
+    expect(await screen.findByText('Apdex')).toBeInTheDocument();
+    expect(await screen.findByRole('button', {name: 'Start Setup'})).toBeInTheDocument();
+    expect(await screen.findByRole('button', {name: 'Get Tour'})).toBeInTheDocument();
+
+    expect(endpointMock).not.toHaveBeenCalled();
   });
 });

+ 123 - 160
static/app/views/projectDetail/projectScoreCards/projectApdexScoreCard.tsx

@@ -3,7 +3,7 @@ import round from 'lodash/round';
 
 import {shouldFetchPreviousPeriod} from 'sentry/components/charts/utils';
 import Count from 'sentry/components/count';
-import DeprecatedAsyncComponent from 'sentry/components/deprecatedAsyncComponent';
+import LoadingError from 'sentry/components/loadingError';
 import {normalizeDateTimeParams} from 'sentry/components/organizations/pageFilters/parse';
 import ScoreCard from 'sentry/components/scoreCard';
 import {parseStatsPeriod} from 'sentry/components/timeRangeSelector/utils';
@@ -14,11 +14,12 @@ import type {Organization} from 'sentry/types/organization';
 import {defined} from 'sentry/utils';
 import type {TableData} from 'sentry/utils/discover/discoverQuery';
 import {getPeriod} from 'sentry/utils/duration/getPeriod';
+import {useApiQuery} from 'sentry/utils/queryClient';
 import {getTermHelp, PerformanceTerm} from 'sentry/views/performance/data';
 
 import MissingPerformanceButtons from '../missingFeatureButtons/missingPerformanceButtons';
 
-type Props = DeprecatedAsyncComponent['props'] & {
+type Props = {
   isProjectStabilized: boolean;
   organization: Organization;
   selection: PageFilters;
@@ -26,187 +27,149 @@ type Props = DeprecatedAsyncComponent['props'] & {
   query?: string;
 };
 
-type State = DeprecatedAsyncComponent['state'] & {
-  currentApdex: TableData | null;
-  previousApdex: TableData | null;
-};
-
-class ProjectApdexScoreCard extends DeprecatedAsyncComponent<Props, State> {
-  shouldRenderBadRequests = true;
-
-  getDefaultState() {
-    return {
-      ...super.getDefaultState(),
-      currentApdex: null,
-      previousApdex: null,
-    };
-  }
-
-  getEndpoints() {
-    const {organization, selection, isProjectStabilized, hasTransactions, query} =
-      this.props;
-
-    if (!this.hasFeature() || !isProjectStabilized || !hasTransactions) {
-      return [];
-    }
-
-    const {projects, environments, datetime} = selection;
-    const {period} = datetime;
-    const commonQuery = {
-      environment: environments,
-      project: projects.map(proj => String(proj)),
-      field: ['apdex()'],
-      query: ['event.type:transaction count():>0', query].join(' ').trim(),
-    };
-    const endpoints: ReturnType<DeprecatedAsyncComponent['getEndpoints']> = [
-      [
-        'currentApdex',
-        `/organizations/${organization.slug}/events/`,
-        {query: {...commonQuery, ...normalizeDateTimeParams(datetime)}},
-      ],
-    ];
-
-    if (
-      shouldFetchPreviousPeriod({
-        start: datetime.start,
-        end: datetime.end,
-        period: datetime.period,
-      })
-    ) {
-      const {start: previousStart} = parseStatsPeriod(
-        getPeriod({period, start: undefined, end: undefined}, {shouldDoublePeriod: true})
-          .statsPeriod!
-      );
-
-      const {start: previousEnd} = parseStatsPeriod(
-        getPeriod({period, start: undefined, end: undefined}, {shouldDoublePeriod: false})
-          .statsPeriod!
-      );
-
-      endpoints.push([
-        'previousApdex',
-        `/organizations/${organization.slug}/events/`,
-        {query: {...commonQuery, start: previousStart, end: previousEnd}},
-      ]);
-    }
-
-    return endpoints;
-  }
-
-  componentDidUpdate(prevProps: Props) {
-    const {selection, isProjectStabilized, hasTransactions, query} = this.props;
-
-    if (
-      prevProps.selection !== selection ||
-      prevProps.hasTransactions !== hasTransactions ||
-      prevProps.isProjectStabilized !== isProjectStabilized ||
-      prevProps.query !== query
-    ) {
-      this.remountComponent();
-    }
-  }
-
-  hasFeature() {
-    return this.props.organization.features.includes('performance-view');
-  }
-
-  get cardTitle() {
-    return t('Apdex');
-  }
-
-  get cardHelp() {
-    const {organization} = this.props;
-    const baseHelp = getTermHelp(organization, PerformanceTerm.APDEX);
-
-    if (this.trend) {
-      return baseHelp + t(' This shows how it has changed since the last period.');
+const useApdex = (props: Props) => {
+  const {organization, selection, isProjectStabilized, hasTransactions, query} = props;
+
+  const isEnabled = !!(
+    organization.features.includes('performance-view') &&
+    isProjectStabilized &&
+    hasTransactions
+  );
+  const {projects, environments: environments, datetime} = selection;
+  const {period} = datetime;
+
+  const {start: previousStart} = parseStatsPeriod(
+    getPeriod({period, start: undefined, end: undefined}, {shouldDoublePeriod: true})
+      .statsPeriod!
+  );
+
+  const {start: previousEnd} = parseStatsPeriod(
+    getPeriod({period, start: undefined, end: undefined}, {shouldDoublePeriod: false})
+      .statsPeriod!
+  );
+
+  const commonQuery = {
+    environment: environments,
+    project: projects.map(proj => String(proj)),
+    field: ['apdex()'],
+    query: ['event.type:transaction count():>0', query].join(' ').trim(),
+  };
+
+  const currentQuery = useApiQuery<TableData>(
+    [
+      `/organizations/${organization.slug}/events/`,
+      {
+        query: {
+          ...commonQuery,
+          ...normalizeDateTimeParams(datetime),
+        },
+      },
+    ],
+    {staleTime: 0, enabled: isEnabled}
+  );
+
+  const isPreviousPeriodEnabled = shouldFetchPreviousPeriod({
+    start: datetime.start,
+    end: datetime.end,
+    period: datetime.period,
+  });
+
+  const previousQuery = useApiQuery<TableData>(
+    [
+      `/organizations/${organization.slug}/events/`,
+      {
+        query: {
+          ...commonQuery,
+          start: previousStart,
+          end: previousEnd,
+        },
+      },
+    ],
+    {
+      staleTime: 0,
+      enabled: isEnabled && isPreviousPeriodEnabled,
     }
+  );
+
+  return {
+    data: currentQuery.data,
+    previousData: previousQuery.data,
+    isLoading:
+      currentQuery.isPending || (previousQuery.isPending && isPreviousPeriodEnabled),
+    error: currentQuery.error || previousQuery.error,
+    refetch: () => {
+      currentQuery.refetch();
+      previousQuery.refetch();
+    },
+  };
+};
 
-    return baseHelp;
-  }
-
-  get currentApdex() {
-    const {currentApdex} = this.state;
-
-    const apdex = currentApdex?.data[0]?.['apdex()'];
-
-    return typeof apdex === 'undefined' ? undefined : Number(apdex);
-  }
+function ProjectApdexScoreCard(props: Props) {
+  const {organization, hasTransactions} = props;
 
-  get previousApdex() {
-    const {previousApdex} = this.state;
+  const {data, previousData, isLoading, error, refetch} = useApdex(props);
 
-    const apdex = previousApdex?.data[0]?.['apdex()'];
+  const apdex = Number(data?.data?.[0]?.['apdex()']) || undefined;
 
-    return typeof apdex === 'undefined' ? undefined : Number(apdex);
-  }
+  const previousApdex = Number(previousData?.data?.[0]['apdex()']) || undefined;
 
-  get trend() {
-    if (this.currentApdex && this.previousApdex) {
-      return round(this.currentApdex - this.previousApdex, 3);
-    }
+  const trend =
+    defined(apdex) && defined(previousApdex)
+      ? round(apdex - previousApdex, 3)
+      : undefined;
 
-    return null;
-  }
+  const shouldRenderTrend = !isLoading && defined(apdex) && defined(trend);
 
-  get trendStatus(): React.ComponentProps<typeof ScoreCard>['trendStatus'] {
-    if (!this.trend) {
-      return undefined;
-    }
+  const cardTitle = t('Apdex');
 
-    return this.trend > 0 ? 'good' : 'bad';
-  }
+  let cardHelp = getTermHelp(organization, PerformanceTerm.APDEX);
 
-  renderLoading() {
-    return this.renderBody();
+  if (trend) {
+    cardHelp += t(' This shows how it has changed since the last period.');
   }
 
-  renderMissingFeatureCard() {
-    const {organization} = this.props;
+  if (!hasTransactions || !organization.features.includes('performance-view')) {
     return (
       <ScoreCard
-        title={this.cardTitle}
-        help={this.cardHelp}
+        title={cardTitle}
+        help={cardHelp}
         score={<MissingPerformanceButtons organization={organization} />}
       />
     );
   }
 
-  renderScore() {
-    return defined(this.currentApdex) ? <Count value={this.currentApdex} /> : '\u2014';
-  }
-
-  renderTrend() {
-    // we want to show trend only after currentApdex has loaded to prevent jumping
-    return defined(this.currentApdex) && defined(this.trend) ? (
-      <Fragment>
-        {this.trend >= 0 ? (
-          <IconArrow direction="up" size="xs" />
-        ) : (
-          <IconArrow direction="down" size="xs" />
-        )}
-        <Count value={Math.abs(this.trend)} />
-      </Fragment>
-    ) : null;
-  }
-
-  renderBody() {
-    const {hasTransactions} = this.props;
-
-    if (!this.hasFeature() || hasTransactions === false) {
-      return this.renderMissingFeatureCard();
-    }
-
+  if (error) {
     return (
-      <ScoreCard
-        title={this.cardTitle}
-        help={this.cardHelp}
-        score={this.renderScore()}
-        trend={this.renderTrend()}
-        trendStatus={this.trendStatus}
+      <LoadingError
+        message={
+          (error.responseJSON?.detail as React.ReactNode) ||
+          t('There was an error loading data.')
+        }
+        onRetry={refetch}
       />
     );
   }
+
+  return (
+    <ScoreCard
+      title={cardTitle}
+      help={cardHelp}
+      score={isLoading || !defined(apdex) ? '\u2014' : <Count value={apdex} />}
+      trend={
+        shouldRenderTrend ? (
+          <Fragment>
+            {trend >= 0 ? (
+              <IconArrow direction="up" size="xs" />
+            ) : (
+              <IconArrow direction="down" size="xs" />
+            )}
+            <Count value={Math.abs(trend)} />
+          </Fragment>
+        ) : null
+      }
+      trendStatus={!trend ? undefined : trend > 0 ? 'good' : 'bad'}
+    />
+  );
 }
 
 export default ProjectApdexScoreCard;