diff --git a/static/app/views/explore/metrics/metricQuery.tsx b/static/app/views/explore/metrics/metricQuery.tsx index 3dcb7a3606f119..5c6ca8db7ce26e 100644 --- a/static/app/views/explore/metrics/metricQuery.tsx +++ b/static/app/views/explore/metrics/metricQuery.tsx @@ -4,6 +4,7 @@ import {defined} from 'sentry/utils'; import type {Sort} from 'sentry/utils/discover/fields'; import {Mode} from 'sentry/views/explore/contexts/pageParamsContext/mode'; import type {AggregateField} from 'sentry/views/explore/queryParams/aggregateField'; +import {validateAggregateSort} from 'sentry/views/explore/queryParams/aggregateSortBy'; import {isGroupBy, type GroupBy} from 'sentry/views/explore/queryParams/groupBy'; import {ReadableQueryParams} from 'sentry/views/explore/queryParams/readableQueryParams'; import { @@ -11,7 +12,6 @@ import { isVisualize, Visualize, VisualizeFunction, - type BaseVisualize, } from 'sentry/views/explore/queryParams/visualize'; export interface TraceMetric { @@ -19,6 +19,14 @@ export interface TraceMetric { type: string; } +function isTraceMetric(value: unknown): value is TraceMetric { + if (value === null || !defined(value) || typeof value !== 'object') { + return false; + } + + return 'name' in value && !!value.name && 'type' in value && !!value.type; +} + export interface BaseMetricQuery { metric: TraceMetric; queryParams: ReadableQueryParams; @@ -38,37 +46,21 @@ export function decodeMetricsQueryParams(value: string): BaseMetricQuery | null return null; } - const metric = json.metric; - if (defined(metric) && typeof metric !== 'object') { - return null; - } - - const query = json.query; - if (typeof query !== 'string') { + if (!isTraceMetric(json.metric)) { return null; } - const rawAggregateFields = json.aggregateFields; - if (!Array.isArray(rawAggregateFields)) { - return null; - } - - const visualizes = rawAggregateFields - .filter(isBaseVisualize) - .flatMap(vis => Visualize.fromJSON(vis)); + const metric = json.metric; + const query = parseQuery(json.query); + const visualizes = parseVisualizes(json.aggregateFields); if (!visualizes.length) { return null; } - const groupBys = rawAggregateFields.filter(isGroupBy); - + const groupBys = parseGroupBys(json.aggregateFields); const aggregateFields = [...visualizes, ...groupBys]; - - const aggregateSortBys = json.aggregateSortBys; - if (!Array.isArray(aggregateSortBys)) { - return null; - } + const aggregateSortBys = parseAggregateSortBys(json.aggregateSortBys, aggregateFields); return { metric, @@ -154,13 +146,24 @@ export function defaultSortBys(fields: string[]): Sort[] { return []; } +function defaultVisualize(): Visualize { + return new VisualizeFunction('per_second(value)'); +} + +function defaultGroupBys(): GroupBy[] { + return []; +} + export function defaultAggregateFields(): AggregateField[] { - return [new VisualizeFunction('per_second(value)')]; + return [defaultVisualize(), ...defaultGroupBys()]; } -export function defaultAggregateSortBys(_: AggregateField[]): Sort[] { - // TODO: Handle aggregate fields for compound sort-by. - return [{field: 'per_second(value)', kind: 'desc'}]; +export function defaultAggregateSortBys(aggregateFields: AggregateField[]): Sort[] { + const visualize = aggregateFields.find(isVisualize); + if (!defined(visualize)) { + return []; + } + return [{field: visualize.yAxis, kind: 'desc'}]; } export function stripMetricParamsFromLocation(location: Location): Location { @@ -173,3 +176,41 @@ export function stripMetricParamsFromLocation(location: Location): Location { return target; } + +function parseQuery(value: unknown): string { + return typeof value === 'string' ? value : defaultQuery(); +} + +function parseVisualizes(value: unknown): Visualize[] { + if (!Array.isArray(value)) { + return []; + } + + const baseVisualize = value.find(isBaseVisualize); + return baseVisualize ? Visualize.fromJSON(baseVisualize) : []; +} + +function parseGroupBys(value: unknown): GroupBy[] { + if (!Array.isArray(value)) { + return defaultGroupBys(); + } + + return value.filter(isGroupBy); +} + +function parseAggregateSortBys( + value: unknown, + aggregateFields: AggregateField[] +): Sort[] { + if (!Array.isArray(value) || value.length === 0) { + return defaultAggregateSortBys(aggregateFields); + } + + if (value.length > 0) { + if (value.some(v => !validateAggregateSort(v, aggregateFields))) { + return defaultAggregateSortBys(aggregateFields); + } + } + + return value; +} diff --git a/static/app/views/explore/queryParams/aggregateSortBy.ts b/static/app/views/explore/queryParams/aggregateSortBy.ts index 6503092548e870..20621ad30e4808 100644 --- a/static/app/views/explore/queryParams/aggregateSortBy.ts +++ b/static/app/views/explore/queryParams/aggregateSortBy.ts @@ -14,7 +14,7 @@ export function getAggregateSortBysFromLocation( const sortBys = decodeSorts(location.query?.[key]); if (sortBys.length > 0) { - if (sortBys.every(sort => validateSort(sort, aggregateFields))) { + if (sortBys.every(sort => validateAggregateSort(sort, aggregateFields))) { return sortBys; } } @@ -22,7 +22,10 @@ export function getAggregateSortBysFromLocation( return null; } -function validateSort(sort: Sort, aggregateFields: AggregateField[]): boolean { +export function validateAggregateSort( + sort: Sort, + aggregateFields: AggregateField[] +): boolean { return aggregateFields.some(aggregateField => { if (isGroupBy(aggregateField)) { return aggregateField.groupBy === sort.field;