Browse Source

ref(decodeSorts): extract and simplify decodeSorts into queryString.tsx (#68848)

EventView is a huge file/class, so i wanted to trim it down a bit and
streamline the `parseSort` functions in the process.

`decode*` is a pattern that already exists inside of queryString.tsx, so
i moved things over to there.

The changes are:
- `eventView.decodeSorts(location)` =>
`queryString.decodeSorts(location.query.sort)`
- `eventView.fromSorts(location.query.sort)` =>
`queryString.decodeSorts(location.query.sort)`
Ryan Albrecht 11 months ago
parent
commit
da47a0b923

+ 2 - 2
static/app/components/replays/useQueryBasedSorting.tsx

@@ -3,8 +3,8 @@ import type {Location} from 'history';
 
 import type {GridColumnOrder} from 'sentry/components/gridEditable';
 import queryBasedSortLinkGenerator from 'sentry/components/replays/queryBasedSortLinkGenerator';
-import {fromSorts} from 'sentry/utils/discover/eventView';
 import type {Sort} from 'sentry/utils/discover/fields';
+import {decodeSorts} from 'sentry/utils/queryString';
 
 interface Props {
   defaultSort: Sort;
@@ -12,7 +12,7 @@ interface Props {
 }
 
 export default function useQueryBasedSorting({location, defaultSort}: Props) {
-  const sorts = useMemo(() => fromSorts(location.query.sort), [location.query.sort]);
+  const sorts = useMemo(() => decodeSorts(location.query.sort), [location.query.sort]);
   const currentSort = useMemo(() => sorts.at(0) ?? defaultSort, [defaultSort, sorts]);
 
   return {

+ 5 - 47
static/app/utils/discover/eventView.tsx

@@ -39,7 +39,7 @@ import {
   DisplayModes,
   TOP_N,
 } from 'sentry/utils/discover/types';
-import {decodeList, decodeScalar} from 'sentry/utils/queryString';
+import {decodeList, decodeScalar, decodeSorts} from 'sentry/utils/queryString';
 import toArray from 'sentry/utils/toArray';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 import type {TableColumn, TableColumnSort} from 'sentry/views/discover/table/types';
@@ -160,48 +160,6 @@ const decodeFields = (location: Location): Array<Field> => {
   return parsed;
 };
 
-const parseSort = (sort: string): Sort => {
-  sort = sort.trim();
-
-  if (sort.startsWith('-')) {
-    return {
-      kind: 'desc',
-      field: sort.substring(1),
-    };
-  }
-
-  return {
-    kind: 'asc',
-    field: sort,
-  };
-};
-
-export const fromSorts = (sorts: string | string[] | undefined): Array<Sort> => {
-  if (sorts === undefined) {
-    return [];
-  }
-
-  sorts = typeof sorts === 'string' ? [sorts] : sorts;
-
-  // NOTE: sets are iterated in insertion order
-  const uniqueSorts = [...new Set(sorts)];
-
-  return uniqueSorts.reduce((acc: Array<Sort>, sort: string) => {
-    acc.push(parseSort(sort));
-    return acc;
-  }, []);
-};
-
-export const decodeSorts = (location: Location): Array<Sort> => {
-  const {query} = location;
-
-  if (!query || !query.sort) {
-    return [];
-  }
-  const sorts = decodeList(query.sort);
-  return fromSorts(sorts);
-};
-
 export const encodeSort = (sort: Sort): string => {
   switch (sort.kind) {
     case 'desc': {
@@ -385,7 +343,7 @@ class EventView {
       id: decodeScalar(location.query.id),
       name: decodeScalar(location.query.name),
       fields: decodeFields(location),
-      sorts: decodeSorts(location),
+      sorts: decodeSorts(location.query.sort),
       query: decodeQuery(location),
       team: decodeTeams(location),
       project: decodeProjects(location),
@@ -473,7 +431,7 @@ class EventView {
       end: decodeScalar(end),
       statsPeriod: decodeScalar(statsPeriod),
       utc,
-      sorts: fromSorts(saved.orderby),
+      sorts: decodeSorts(saved.orderby),
       environment: collectQueryStringByKey(
         {
           environment: saved.environment as string[],
@@ -502,7 +460,7 @@ class EventView {
     const id = decodeScalar(location.query.id);
     const teams = decodeTeams(location);
     const projects = decodeProjects(location);
-    const sorts = decodeSorts(location);
+    const sorts = decodeSorts(location.query.sort);
     const environments = collectQueryStringByKey(location.query, 'environment');
 
     if (saved) {
@@ -531,7 +489,7 @@ class EventView {
           'query' in location.query
             ? decodeQuery(location)
             : queryStringFromSavedQuery(saved),
-        sorts: sorts.length === 0 ? fromSorts(saved.orderby) : sorts,
+        sorts: sorts.length === 0 ? decodeSorts(saved.orderby) : sorts,
         yAxis:
           decodeScalar(location.query.yAxis) ||
           // Workaround to only use the first yAxis since eventView yAxis doesn't accept string[]

+ 37 - 0
static/app/utils/queryString.spec.tsx

@@ -170,3 +170,40 @@ describe('decodeInteger()', function () {
     expect(utils.decodeInteger('', 2020)).toEqual(2020);
   });
 });
+
+describe('decodeSorts', () => {
+  it('handles simple strings and lists', () => {
+    expect(utils.decodeSorts('startedAt')).toEqual([{kind: 'asc', field: 'startedAt'}]);
+    expect(utils.decodeSorts(['startedAt', 'finishedAt'])).toEqual([
+      {kind: 'asc', field: 'startedAt'},
+      {kind: 'asc', field: 'finishedAt'},
+    ]);
+    expect(utils.decodeSorts('-startedAt')).toEqual([{kind: 'desc', field: 'startedAt'}]);
+    expect(utils.decodeSorts(['-startedAt', '-finishedAt'])).toEqual([
+      {kind: 'desc', field: 'startedAt'},
+      {kind: 'desc', field: 'finishedAt'},
+    ]);
+  });
+
+  it('handles falsey values', () => {
+    expect(utils.decodeSorts(null)).toEqual([]);
+    expect(utils.decodeSorts(undefined)).toEqual([]);
+    expect(utils.decodeSorts('')).toEqual([]);
+    expect(utils.decodeSorts([''])).toEqual([]);
+  });
+
+  it('fallsback to a default value', () => {
+    expect(utils.decodeSorts(null, '-startedAt')).toEqual([
+      {kind: 'desc', field: 'startedAt'},
+    ]);
+    expect(utils.decodeSorts(undefined, '-startedAt')).toEqual([
+      {kind: 'desc', field: 'startedAt'},
+    ]);
+    expect(utils.decodeSorts('', '-startedAt')).toEqual([
+      {kind: 'desc', field: 'startedAt'},
+    ]);
+    expect(utils.decodeSorts([''], '-startedAt')).toEqual([
+      {kind: 'desc', field: 'startedAt'},
+    ]);
+  });
+});

+ 16 - 0
static/app/utils/queryString.tsx

@@ -1,6 +1,7 @@
 import * as qs from 'query-string';
 
 import {escapeDoubleQuotes} from 'sentry/utils';
+import type {Sort} from 'sentry/utils/discover/fields';
 import {safeURL} from 'sentry/utils/url/safeURL';
 
 // remove leading and trailing whitespace and remove double spaces
@@ -125,10 +126,25 @@ export function decodeInteger(value: QueryValue, fallback?: number): number | un
   return fallback;
 }
 
+export function decodeSorts(value: QueryValue): Sort[];
+export function decodeSorts(value: QueryValue, fallback: string): Sort[];
+export function decodeSorts(value: QueryValue, fallback?: string): Sort[] {
+  const sorts = decodeList(value).filter(Boolean);
+  if (!sorts.length) {
+    return fallback ? decodeSorts(fallback) : [];
+  }
+  return sorts.map(sort =>
+    sort.startsWith('-')
+      ? {kind: 'desc', field: sort.substring(1)}
+      : {kind: 'asc', field: sort}
+  );
+}
+
 const queryString = {
   decodeInteger,
   decodeList,
   decodeScalar,
+  decodeSorts,
   formatQueryString,
   addQueryParamsToExistingUrl,
   appendTagCondition,

+ 5 - 2
static/app/views/issueDetails/allEventsTable.tsx

@@ -10,11 +10,12 @@ import {
 import {t} from 'sentry/locale';
 import type {EventTransaction, Group, Organization, PlatformKey} from 'sentry/types';
 import {IssueCategory, IssueType} from 'sentry/types';
-import EventView, {decodeSorts} from 'sentry/utils/discover/eventView';
+import EventView from 'sentry/utils/discover/eventView';
 import {DiscoverDatasets} from 'sentry/utils/discover/types';
 import {getConfigForIssueType} from 'sentry/utils/issueTypeConfig';
 import {platformToCategory} from 'sentry/utils/platform';
 import {useApiQuery} from 'sentry/utils/queryClient';
+import {decodeSorts} from 'sentry/utils/queryString';
 import {projectCanLinkToReplay} from 'sentry/utils/replays/projectSupportsReplay';
 import {useRoutes} from 'sentry/utils/useRoutes';
 import EventsTable from 'sentry/views/performance/transactionSummary/transactionEvents/eventsTable';
@@ -63,7 +64,9 @@ function AllEventsTable(props: Props) {
   }
   eventView.fields = fields.map(fieldName => ({field: fieldName}));
 
-  eventView.sorts = decodeSorts(location).filter(sort => fields.includes(sort.field));
+  eventView.sorts = decodeSorts(location.query.sort).filter(sort =>
+    fields.includes(sort.field)
+  );
 
   useEffect(() => {
     setError('');

+ 4 - 2
static/app/views/performance/browser/resources/utils/useResourceSort.ts

@@ -1,5 +1,5 @@
-import {fromSorts} from 'sentry/utils/discover/eventView';
 import type {Sort} from 'sentry/utils/discover/fields';
+import {decodeSorts} from 'sentry/utils/queryString';
 import {useLocation} from 'sentry/utils/useLocation';
 import {SpanFunction, SpanMetricsField} from 'sentry/views/starfish/types';
 import type {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
@@ -33,7 +33,9 @@ export function useResourceSort(
 ) {
   const location = useLocation<Query>();
 
-  return fromSorts(location.query[sortParameterName]).filter(isAValidSort)[0] ?? fallback;
+  return (
+    decodeSorts(location.query[sortParameterName]).filter(isAValidSort)[0] ?? fallback
+  );
 }
 
 const DEFAULT_SORT: Sort = {

+ 4 - 2
static/app/views/performance/browser/resources/utils/useResourceSummarySort.ts

@@ -1,5 +1,5 @@
-import {fromSorts} from 'sentry/utils/discover/eventView';
 import type {Sort} from 'sentry/utils/discover/fields';
+import {decodeSorts} from 'sentry/utils/queryString';
 import {useLocation} from 'sentry/utils/useLocation';
 import {SpanMetricsField} from 'sentry/views/starfish/types';
 import type {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
@@ -30,7 +30,9 @@ export function useResourceSummarySort(
 ) {
   const location = useLocation<Query>();
 
-  return fromSorts(location.query[sortParameterName]).filter(isAValidSort)[0] ?? fallback;
+  return (
+    decodeSorts(location.query[sortParameterName]).filter(isAValidSort)[0] ?? fallback
+  );
 }
 
 const DEFAULT_SORT: Sort = {

+ 4 - 2
static/app/views/performance/browser/useBrowserSort.ts

@@ -1,5 +1,5 @@
-import {fromSorts} from 'sentry/utils/discover/eventView';
 import type {Sort} from 'sentry/utils/discover/fields';
+import {decodeSorts} from 'sentry/utils/queryString';
 import {useLocation} from 'sentry/utils/useLocation';
 import type {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
 
@@ -23,7 +23,9 @@ export function useBrowserSort(
 ) {
   const location = useLocation<Query>();
 
-  return fromSorts(location.query[sortParameterName]).filter(isAValidSort)[0] ?? fallback;
+  return (
+    decodeSorts(location.query[sortParameterName]).filter(isAValidSort)[0] ?? fallback
+  );
 }
 
 const DEFAULT_SORT: Sort = {

+ 3 - 4
static/app/views/performance/browser/webVitals/utils/useWebVitalsSort.tsx

@@ -1,6 +1,5 @@
-import {fromSorts} from 'sentry/utils/discover/eventView';
 import type {Sort} from 'sentry/utils/discover/fields';
-import {decodeScalar} from 'sentry/utils/queryString';
+import {decodeSorts} from 'sentry/utils/queryString';
 import {useLocation} from 'sentry/utils/useLocation';
 import {
   DEFAULT_SORT,
@@ -20,8 +19,8 @@ export function useWebVitalsSort({
   const filteredSortableFields = sortableFields;
 
   const sort =
-    fromSorts(decodeScalar(location.query[sortName])).filter(s =>
-      (filteredSortableFields as unknown as string[]).includes(s.field)
+    decodeSorts(location.query[sortName]).filter(s =>
+      filteredSortableFields.includes(s.field)
     )[0] ?? defaultSort;
 
   return sort;

+ 2 - 3
static/app/views/performance/database/databaseLandingPage.tsx

@@ -13,8 +13,7 @@ import {ProjectPageFilter} from 'sentry/components/organizations/projectPageFilt
 import SearchBar from 'sentry/components/searchBar';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import {fromSorts} from 'sentry/utils/discover/eventView';
-import {decodeScalar} from 'sentry/utils/queryString';
+import {decodeScalar, decodeSorts} from 'sentry/utils/queryString';
 import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
@@ -49,7 +48,7 @@ export function DatabaseLandingPage() {
 
   const sortField = decodeScalar(location.query?.[QueryParameterNames.SPANS_SORT]);
 
-  let sort = fromSorts(sortField).filter(isAValidSort)[0];
+  let sort = decodeSorts(sortField).filter(isAValidSort)[0];
   if (!sort) {
     sort = DEFAULT_SORT;
   }

Some files were not shown because too many files changed in this diff