feat(starfish): make table sorting consistent, fix asc/desc sort in module and span summary (#51801)

static/app/views/starfish/components/tableCells/renderHeadCell.tsx

@@ -25,12 +25,22 @@ export const SORTABLE_FIELDS = new Set([
   'sps()',
   'sps_percent_change()',
   'time_spent_percentage()',
+  'time_spent_percentage(local)',
 ]);
 
 export const renderHeadCell = ({column, location, sort}: Options) => {
   const {key, name} = column;
   const alignment = getAlignment(key);
 
+  let newSortDirection: Sort['kind'] = 'desc';
+  if (sort?.field === column.key) {
+    if (sort.kind === 'desc') {
+      newSortDirection = 'asc';
+    }
+  }
+
+  const newSort = `${newSortDirection === 'desc' ? '-' : ''}${key}`;
+
   return (
     <SortLink
       align={alignment}
@@ -42,7 +52,7 @@ export const renderHeadCell = ({column, location, sort}: Options) => {
           ...location,
           query: {
             ...location?.query,
-            [QueryParameterNames.SORT]: `-${key}`,
+            [QueryParameterNames.SORT]: newSort,
           },
         };
       }}

static/app/views/starfish/queries/useSpanTransactionMetrics.tsx

@@ -1,6 +1,7 @@
 import {Location} from 'history';
 
 import EventView from 'sentry/utils/discover/eventView';
+import {Sort} from 'sentry/utils/discover/fields';
 import {DiscoverDatasets} from 'sentry/utils/discover/types';
 import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import {useLocation} from 'sentry/utils/useLocation';
@@ -24,12 +25,14 @@ export type SpanTransactionMetrics = {
 
 export const useSpanTransactionMetrics = (
   span: Pick<IndexedSpan, 'group'>,
-  transactions?: string[],
+  options: {sorts?: Sort[]; transactions?: string[]},
   _referrer = 'span-transaction-metrics'
 ) => {
   const location = useLocation();
 
-  const eventView = getEventView(span, location, transactions ?? []);
+  const {transactions, sorts} = options;
+
+  const eventView = getEventView(span, location, transactions ?? [], sorts);
 
   return useWrappedDiscoverQuery<SpanTransactionMetrics[]>({
     eventView,
@@ -38,7 +41,12 @@ export const useSpanTransactionMetrics = (
   });
 };
 
-function getEventView(span: {group: string}, location: Location, transactions: string[]) {
+function getEventView(
+  span: {group: string},
+  location: Location,
+  transactions: string[],
+  sorts?: Sort[]
+) {
   const cleanGroupId = span.group.replaceAll('-', '').slice(-16);
 
   const search = new MutableSearch('');
@@ -49,7 +57,7 @@ function getEventView(span: {group: string}, location: Location, transactions: s
     search.addFilterValues('transaction', transactions);
   }
 
-  return EventView.fromNewQueryWithLocation(
+  const eventView = EventView.fromNewQueryWithLocation(
     {
       name: '',
       query: search.formatString(),
@@ -71,4 +79,10 @@ function getEventView(span: {group: string}, location: Location, transactions: s
     },
     location
   );
+
+  if (sorts) {
+    eventView.sorts = sorts;
+  }
+
+  return eventView;
 }

static/app/views/starfish/views/spanSummaryPage/index.tsx

@@ -11,6 +11,8 @@ import {Panel, PanelBody} from 'sentry/components/panels';
 import QuestionTooltip from 'sentry/components/questionTooltip';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
+import {fromSorts} from 'sentry/utils/discover/eventView';
+import {Sort} from 'sentry/utils/discover/fields';
 import {
   PageErrorAlert,
   PageErrorProvider,
@@ -31,12 +33,21 @@ import {SpanMetricsFields} from 'sentry/views/starfish/types';
 import formatThroughput from 'sentry/views/starfish/utils/chartValueFormatters/formatThroughput';
 import {extractRoute} from 'sentry/views/starfish/utils/extractRoute';
 import {ROUTE_NAMES} from 'sentry/views/starfish/utils/routeNames';
+import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
 import {DataTitles} from 'sentry/views/starfish/views/spans/types';
 import {SampleList} from 'sentry/views/starfish/views/spanSummaryPage/sampleList';
-import {SpanTransactionsTable} from 'sentry/views/starfish/views/spanSummaryPage/spanTransactionsTable';
+import {
+  isAValidSort,
+  SpanTransactionsTable,
+} from 'sentry/views/starfish/views/spanSummaryPage/spanTransactionsTable';
 
 const {SPAN_SELF_TIME} = SpanMetricsFields;
 
+const DEFAULT_SORT: Sort = {
+  kind: 'desc',
+  field: 'time_spent_percentage(local)',
+};
+
 type Props = {
   location: Location;
 } & RouteComponentProps<{groupId: string}, {transaction: string}>;
@@ -47,6 +58,9 @@ function SpanSummaryPage({params, location}: Props) {
   const {transaction, transactionMethod, endpoint, endpointMethod} = location.query;
 
   const queryFilter = endpoint ? {transactionName: endpoint} : undefined;
+  const sort =
+    fromSorts(location.query[QueryParameterNames.SORT]).filter(isAValidSort)[0] ??
+    DEFAULT_SORT; // We only allow one sort on this table in this view
 
   if (endpointMethod && queryFilter) {
     queryFilter['transaction.method'] = endpointMethod;
@@ -225,6 +239,7 @@ function SpanSummaryPage({params, location}: Props) {
               {span && (
                 <SpanTransactionsTable
                   span={span}
+                  sort={sort}
                   endpoint={endpoint}
                   endpointMethod={endpointMethod}
                 />

static/app/views/starfish/views/spanSummaryPage/spanTransactionsTable.tsx

@@ -14,9 +14,13 @@ import Pagination from 'sentry/components/pagination';
 import Truncate from 'sentry/components/truncate';
 import {t} from 'sentry/locale';
 import {getFieldRenderer} from 'sentry/utils/discover/fieldRenderers';
+import {Sort} from 'sentry/utils/discover/fields';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
-import {renderHeadCell} from 'sentry/views/starfish/components/tableCells/renderHeadCell';
+import {
+  renderHeadCell,
+  SORTABLE_FIELDS,
+} from 'sentry/views/starfish/components/tableCells/renderHeadCell';
 import type {IndexedSpan} from 'sentry/views/starfish/queries/types';
 import {
   SpanTransactionMetrics,
@@ -33,6 +37,7 @@ type Row = {
 };
 
 type Props = {
+  sort: ValidSort;
   span: Pick<IndexedSpan, 'group'>;
   endpoint?: string;
   endpointMethod?: string;
@@ -40,6 +45,10 @@ type Props = {
   openSidebar?: boolean;
 };
 
+type ValidSort = Sort & {
+  field: keyof Row;
+};
+
 export type TableColumnHeader = GridColumnHeader<keyof Row['metrics']>;
 
 export function SpanTransactionsTable({
@@ -48,6 +57,7 @@ export function SpanTransactionsTable({
   onClickTransaction,
   endpoint,
   endpointMethod,
+  sort,
 }: Props) {
   const location = useLocation();
   const organization = useOrganization();
@@ -57,7 +67,10 @@ export function SpanTransactionsTable({
     meta,
     isLoading,
     pageLinks,
-  } = useSpanTransactionMetrics(span, endpoint ? [endpoint] : undefined);
+  } = useSpanTransactionMetrics(span, {
+    transactions: endpoint ? [endpoint] : undefined,
+    sorts: [sort],
+  });
 
   const spanTransactionsWithMetrics = spanTransactionMetrics.map(row => {
     return {
@@ -105,7 +118,7 @@ export function SpanTransactionsTable({
         columnOrder={COLUMN_ORDER}
         columnSortBy={[]}
         grid={{
-          renderHeadCell: col => renderHeadCell({column: col}),
+          renderHeadCell: col => renderHeadCell({column: col, sort, location}),
           renderBodyCell,
         }}
         location={location}
@@ -202,3 +215,7 @@ const StyledPagination = styled(Pagination)`
   margin-top: 0;
   margin-left: auto;
 `;
+
+export function isAValidSort(sort: Sort): sort is ValidSort {
+  return SORTABLE_FIELDS.has(sort.field);
+}

static/app/views/starfish/views/spans/spansView.tsx

@@ -18,7 +18,7 @@ import SpansTable, {isAValidSort} from './spansTable';
 
 const DEFAULT_SORT: Sort = {
   kind: 'desc',
-  field: 'sps()',
+  field: 'time_spent_percentage()',
 };
 const LIMIT: number = 25;