Browse Source

ref(replays): Refactor replay pagination to use `Link` response header (#37960)

Depends on: #37945 

Fixes #37931
Ryan Albrecht 2 years ago
parent
commit
7c833a783a

+ 4 - 73
static/app/utils/replays/hooks/useReplayList.tsx

@@ -1,11 +1,7 @@
 import {useCallback, useEffect, useState} from 'react';
 import * as Sentry from '@sentry/react';
-import type {Location} from 'history';
-import {stringify} from 'query-string';
 
 import type {Organization} from 'sentry/types';
-import {defined} from 'sentry/utils';
-import {getLocalToSystem, getUserTimezone, getUtcDateString} from 'sentry/utils/dates';
 import EventView from 'sentry/utils/discover/eventView';
 import {decodeInteger} from 'sentry/utils/queryString';
 import {mapResponseToReplayRecord} from 'sentry/utils/replays/replayDataUtils';
@@ -71,24 +67,14 @@ function useReplayList({
       const path = `/organizations/${organization.slug}/replays/`;
       const query = eventView.generateQueryStringObject();
 
-      // TODO(replays): Need to add one as a sentinel value to detect if there
-      // are more pages. Shouldn't need this when response has pageLinks header.
-      query.limit = String(queryLimit + 1);
+      query.limit = String(queryLimit);
       query.offset = String(queryOffset);
-      const response = await api.requestPromise(path, {
+      const [{data: records}, _textStatus, resp] = await api.requestPromise(path, {
+        includeAllArgs: true,
         query: eventView.getEventsAPIPayload(location),
       });
 
-      // TODO(replays): Remove the `slice()` call once `pageLinks` is in response headers.
-      const records = response.data.slice(0, queryLimit);
-
-      // TODO(replays): Response should include pageLinks header instead of this.
-      const pageLinks = getPageLinks({
-        defaultLimit,
-        query: eventView.generateQueryStringObject(),
-        path,
-        records: response.data,
-      });
+      const pageLinks = resp?.getResponseHeader('Link') ?? '';
 
       setData({
         fetchError: undefined,
@@ -114,59 +100,4 @@ function useReplayList({
   return data;
 }
 
-// We should be getting pageLinks as response headers, not constructing them.
-function getPageLinks({
-  defaultLimit,
-  path,
-  query,
-  records,
-}: {
-  defaultLimit: number;
-  path: string;
-  query: Location<ReplayListLocationQuery>['query'];
-  records: unknown[];
-}) {
-  // Remove extra fields that EventView uses internally
-  Object.keys(query).forEach(key => {
-    if (!defined(query[key]) || query[key] === '') {
-      delete query[key];
-    }
-  });
-
-  // Add & subtract one because we added one above as a sentinel to tell if there is a next page
-  const queryLimit = decodeInteger(query.limit, defaultLimit);
-  const queryOffset = decodeInteger(query.offset, 0);
-
-  const prevOffset = queryOffset - queryLimit;
-  const nextOffset = queryOffset + queryLimit;
-
-  const hasPrev = prevOffset >= 0;
-  const hasNext = records.length === queryLimit;
-
-  const utc =
-    query.utc === 'true'
-      ? true
-      : query.utc === 'false'
-      ? false
-      : getUserTimezone() === 'UTC';
-
-  const qs = stringify({
-    ...query,
-    limit: String(queryLimit),
-    offset: String(queryOffset),
-    start: getUtcDateString(utc ? query.start : getLocalToSystem(query.start)),
-    end: getUtcDateString(utc ? query.end : getLocalToSystem(query.end)),
-  });
-  const url = `${path}?${qs}`;
-
-  return [
-    hasPrev
-      ? `<${url}>; rel="previous"; cursor="${prevOffset}"`
-      : `<${url}>; rel="previous"; results="false"`,
-    hasNext
-      ? `<${url}>; rel="next"; cursor="${nextOffset}"`
-      : `<${url}>; rel="next"; results="false" `,
-  ].join(',');
-}
-
 export default useReplayList;

+ 26 - 1
static/app/views/replays/replays.tsx

@@ -3,6 +3,7 @@ import {browserHistory, RouteComponentProps} from 'react-router';
 import {useTheme} from '@emotion/react';
 import styled from '@emotion/styled';
 
+import DetailedError from 'sentry/components/errors/detailedError';
 import PageFiltersContainer from 'sentry/components/organizations/pageFilters/container';
 import PageHeading from 'sentry/components/pageHeading';
 import Pagination from 'sentry/components/pagination';
@@ -44,11 +45,35 @@ function Replays({location}: Props) {
   }, [location]);
 
   const {pathname, query} = location;
-  const {replays, pageLinks, isFetching} = useReplayList({
+  const {replays, pageLinks, isFetching, fetchError} = useReplayList({
     organization,
     eventView,
   });
 
+  if (fetchError && !isFetching) {
+    const reasons = [
+      t('The search parameters you selected are invalid in some way'),
+      t('There is an internal systems error or active issue'),
+    ];
+
+    return (
+      <DetailedError
+        hideSupportLinks
+        heading={t('Sorry, the list of replays could not be found.')}
+        message={
+          <div>
+            <p>{t('This could be due to a handful of reasons:')}</p>
+            <ol className="detailed-error-list">
+              {reasons.map((reason, i) => (
+                <li key={i}>{reason}</li>
+              ))}
+            </ol>
+          </div>
+        }
+      />
+    );
+  }
+
   return (
     <Fragment>
       <StyledPageHeader>