Browse Source

fix(metrics): Uppercase for new styles only (#74961)

Priscila Oliveira 7 months ago
parent
commit
0067c2809f

+ 1 - 1
static/app/components/metrics/equationInput/index.tsx

@@ -56,7 +56,7 @@ export function EquationInput({
 
   const validateVariable = useCallback(
     (variable: string): string | null => {
-      if (!availableVariables.has(variable.toUpperCase())) {
+      if (!availableVariables.has(variable)) {
         return t('Unknown query "%s"', variable);
       }
       return null;

+ 14 - 3
static/app/components/metrics/equationSymbol.spec.tsx

@@ -8,17 +8,28 @@ import {
 
 describe('getEquationSymbol', () => {
   it('should return the correct symbol', () => {
-    expect(getEquationSymbol(0)).toBe('Ƒ1');
-    expect(getEquationSymbol(1)).toBe('Ƒ2');
+    expect(getEquationSymbol(0)).toBe('ƒ1');
+    expect(getEquationSymbol(1)).toBe('ƒ2');
   });
 });
 
 describe('EquationSymbol', () => {
   it('renders', () => {
     render(<EquationSymbol equationId={0} />);
-    expect(screen.getByText(textWithMarkupMatcher('Ƒ1'))).toBeInTheDocument();
+    expect(screen.getByText(textWithMarkupMatcher('ƒ1'))).toBeInTheDocument();
 
     render(<EquationSymbol equationId={5} />);
+    expect(screen.getByText(textWithMarkupMatcher('ƒ6'))).toBeInTheDocument();
+  });
+  it('renders uppercase', () => {
+    render(<EquationSymbol equationId={0} />, {
+      organization: {features: ['metrics-new-inputs']},
+    });
+    expect(screen.getByText(textWithMarkupMatcher('Ƒ1'))).toBeInTheDocument();
+
+    render(<EquationSymbol equationId={5} />, {
+      organization: {features: ['metrics-new-inputs']},
+    });
     expect(screen.getByText(textWithMarkupMatcher('Ƒ6'))).toBeInTheDocument();
   });
 });

+ 20 - 6
static/app/components/metrics/equationSymbol.tsx

@@ -9,20 +9,34 @@ interface EquationSymbolProps extends React.ComponentProps<typeof DeprecatedSymb
   equationId: number;
 }
 
-export function getEquationSymbol(equationId: number) {
-  return `Ƒ${equationId + 1}`;
+export function getEquationSymbol(
+  equationId: number,
+  metricsNewInputs?: boolean
+): string {
+  if (metricsNewInputs) {
+    return `Ƒ${equationId + 1}`;
+  }
+  return `ƒ${equationId + 1}`;
 }
 
 export const EquationSymbol = forwardRef<HTMLSpanElement, EquationSymbolProps>(
   function EquationSymbol({equationId, ...props}, ref) {
     const organization = useOrganization();
-    const Component = hasMetricsNewInputs(organization) ? Symbol : DeprecatedSymbol;
+    if (hasMetricsNewInputs(organization)) {
+      return (
+        <Symbol ref={ref} {...props}>
+          <span>
+            Ƒ<sub>{equationId + 1}</sub>
+          </span>
+        </Symbol>
+      );
+    }
     return (
-      <Component ref={ref} {...props}>
+      <DeprecatedSymbol ref={ref} {...props}>
         <span>
-          Ƒ<sub>{equationId + 1}</sub>
+          ƒ<sub>{equationId + 1}</sub>
         </span>
-      </Component>
+      </DeprecatedSymbol>
     );
   }
 );

+ 24 - 12
static/app/components/metrics/querySymbol.spec.tsx

@@ -4,30 +4,42 @@ import {getQuerySymbol, QuerySymbol} from 'sentry/components/metrics/querySymbol
 
 describe('getQuerySymbol', () => {
   it('should return the correct symbol', () => {
-    expect(getQuerySymbol(0)).toBe('A');
-    expect(getQuerySymbol(1)).toBe('B');
-    expect(getQuerySymbol(25)).toBe('Z');
-    expect(getQuerySymbol(26)).toBe('AA');
-    expect(getQuerySymbol(27)).toBe('AB');
-    expect(getQuerySymbol(52)).toBe('BA');
-    expect(getQuerySymbol(53)).toBe('BB');
-    expect(getQuerySymbol(77)).toBe('BZ');
-    expect(getQuerySymbol(78)).toBe('CA');
-    expect(getQuerySymbol(702)).toBe('AAA');
+    expect(getQuerySymbol(0)).toBe('a');
+    expect(getQuerySymbol(1)).toBe('b');
+    expect(getQuerySymbol(25)).toBe('z');
+    expect(getQuerySymbol(26)).toBe('aa');
+    expect(getQuerySymbol(27)).toBe('ab');
+    expect(getQuerySymbol(52)).toBe('ba');
+    expect(getQuerySymbol(53)).toBe('bb');
+    expect(getQuerySymbol(77)).toBe('bz');
+    expect(getQuerySymbol(78)).toBe('ca');
+    expect(getQuerySymbol(702)).toBe('aaa');
   });
 });
 
 describe('QuerySymbol', () => {
   it('renders', () => {
     render(<QuerySymbol queryId={0} />);
-    expect(screen.getByText('A')).toBeInTheDocument();
+    expect(screen.getByText('a')).toBeInTheDocument();
 
     render(<QuerySymbol queryId={27} />);
-    expect(screen.getByText('AB')).toBeInTheDocument();
+    expect(screen.getByText('ab')).toBeInTheDocument();
   });
 
   it('does not render for negative query ids', () => {
     const {container} = render(<QuerySymbol queryId={-1} />);
     expect(container).toBeEmptyDOMElement();
   });
+
+  it('renders everything in uppercase', () => {
+    render(<QuerySymbol queryId={0} />, {
+      organization: {features: ['metrics-new-inputs']},
+    });
+    expect(screen.getByText('A')).toBeInTheDocument();
+
+    render(<QuerySymbol queryId={27} />, {
+      organization: {features: ['metrics-new-inputs']},
+    });
+    expect(screen.getByText('AB')).toBeInTheDocument();
+  });
 });

+ 13 - 9
static/app/components/metrics/querySymbol.tsx

@@ -5,22 +5,21 @@ import {space} from 'sentry/styles/space';
 import {hasMetricsNewInputs} from 'sentry/utils/metrics/features';
 import useOrganization from 'sentry/utils/useOrganization';
 
-const indexToChar = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ';
+const indexToChar = 'abcdefghijklmnopqrstuvwxyz';
 
-export const getQuerySymbol = (index: number) => {
+export const getQuerySymbol = (index: number, uppercaseChar?: boolean) => {
   let result = '';
   let i = index;
   do {
     result = indexToChar[i % indexToChar.length] + result;
     i = Math.floor(i / indexToChar.length) - 1;
   } while (i >= 0);
-  return result;
+  return uppercaseChar ? result.toUpperCase() : result;
 };
 
 export const DeprecatedSymbol = styled('span')<{
   isHidden?: boolean;
 }>`
-  text-transform: lowercase;
   display: flex;
   width: 38px;
   height: 38px;
@@ -47,7 +46,6 @@ export const Symbol = styled(DeprecatedSymbol)`
   color: ${p => p.theme.purple300};
   border: 1px solid ${p => p.theme.purple200};
   background: ${p => p.theme.purple100};
-  text-transform: uppercase;
   font-weight: 600;
 `;
 
@@ -63,12 +61,18 @@ export const QuerySymbol = forwardRef<HTMLSpanElement, QuerySymbolProps>(
       return null;
     }
 
-    const Component = hasMetricsNewInputs(organization) ? Symbol : DeprecatedSymbol;
+    if (hasMetricsNewInputs(organization)) {
+      return (
+        <Symbol ref={ref} {...props}>
+          <span>{getQuerySymbol(queryId, true)}</span>
+        </Symbol>
+      );
+    }
 
     return (
-      <Component ref={ref} {...props}>
-        <span>{getQuerySymbol(queryId)}</span>
-      </Component>
+      <DeprecatedSymbol ref={ref} {...props}>
+        <span>{getQuerySymbol(queryId, false)}</span>
+      </DeprecatedSymbol>
     );
   }
 );

+ 4 - 2
static/app/components/modals/metricWidgetViewerModal/queries.tsx

@@ -66,10 +66,12 @@ export const Queries = memo(function Queries({
   removeEquation,
 }: Props) {
   const {selection} = usePageFilters();
+  const organization = useOrganization();
+  const metricsNewInputs = hasMetricsNewInputs(organization);
 
   const availableVariables = useMemo(
-    () => new Set(metricQueries.map(query => getQuerySymbol(query.id))),
-    [metricQueries]
+    () => new Set(metricQueries.map(query => getQuerySymbol(query.id, metricsNewInputs))),
+    [metricQueries, metricsNewInputs]
   );
 
   const handleEditQueryAlias = useCallback(

+ 8 - 2
static/app/components/modals/metricWidgetViewerModal/visualization.tsx

@@ -11,6 +11,7 @@ import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import type {MetricsQueryApiResponse} from 'sentry/types/metrics';
 import {DEFAULT_SORT_STATE} from 'sentry/utils/metrics/constants';
+import {hasMetricsNewInputs} from 'sentry/utils/metrics/features';
 import {parseMRI} from 'sentry/utils/metrics/mri';
 import {
   type FocusedMetricsSeries,
@@ -21,6 +22,7 @@ import {
   type MetricsQueryApiQueryParams,
   useMetricsQuery,
 } from 'sentry/utils/metrics/useMetricsQuery';
+import useOrganization from 'sentry/utils/useOrganization';
 import usePageFilters from 'sentry/utils/usePageFilters';
 import {DASHBOARD_CHART_GROUP} from 'sentry/views/dashboards/dashboard';
 import {BigNumber, getBigNumberData} from 'sentry/views/dashboards/metrics/bigNumber';
@@ -133,6 +135,8 @@ export function MetricVisualization({
   interval,
 }: MetricVisualizationProps) {
   const {selection} = usePageFilters();
+  const organization = useOrganization();
+  const metricsNewInputs = hasMetricsNewInputs(organization);
   const hasSetMetric = useMemo(
     () =>
       expressions.some(
@@ -149,7 +153,10 @@ export function MetricVisualization({
     onIntervalChange: EMPTY_FN,
   });
 
-  const queries = useMemo(() => expressionsToApiQueries(expressions), [expressions]);
+  const queries = useMemo(
+    () => expressionsToApiQueries(expressions, metricsNewInputs),
+    [expressions, metricsNewInputs]
+  );
 
   const {
     data: timeseriesData,
@@ -332,7 +339,6 @@ function MetricChartVisualization({
         height={200}
       />
       <SummaryTable
-        singleQuery={queries.length === 1}
         series={chartSeries}
         onSortChange={setTableSort}
         sort={tableSort}

+ 46 - 12
static/app/utils/metrics/dashboardImport.spec.tsx

@@ -1,3 +1,5 @@
+import {OrganizationFixture} from 'sentry-fixture/organization';
+
 import type {MetricMeta, MRI} from 'sentry/types/metrics';
 import type {ImportDashboard, ImportWidget} from 'sentry/utils/metrics/dashboardImport';
 import {parseDashboard, WidgetParser} from 'sentry/utils/metrics/dashboardImport';
@@ -74,7 +76,12 @@ describe('WidgetParser', () => {
   it('should parse a widget with single timeseries', async () => {
     const widgetToImport = mockWidget();
 
-    const result = new WidgetParser(widgetToImport, availableMetrics, 'test-org').parse();
+    const result = new WidgetParser(
+      widgetToImport,
+      availableMetrics,
+      'test-org',
+      false
+    ).parse();
     const {report, widget} = await result;
 
     expect(report.outcome).toEqual('success');
@@ -106,7 +113,12 @@ describe('WidgetParser', () => {
         'sum:sentry.bar.baz{}',
       ]),
     });
-    const result = new WidgetParser(widgetToImport, availableMetrics, 'test-org').parse();
+    const result = new WidgetParser(
+      widgetToImport,
+      availableMetrics,
+      'test-org',
+      false
+    ).parse();
     const {report, widget} = await result;
 
     expect(report.outcome).toEqual('success');
@@ -145,7 +157,12 @@ describe('WidgetParser', () => {
       requests: mockRequests(['sum:sentry.foo.bar.avg{foo:bar} by {baz}']),
     });
 
-    const result = new WidgetParser(widgetToImport, availableMetrics, 'test-org').parse();
+    const result = new WidgetParser(
+      widgetToImport,
+      availableMetrics,
+      'test-org',
+      false
+    ).parse();
     const {report, widget} = await result;
 
     expect(report.outcome).toEqual('success');
@@ -172,7 +189,12 @@ describe('WidgetParser', () => {
       requests: mockRequests(['sum:sentry.unknown-metric{foo:bar} by {baz}']),
     });
 
-    const result = new WidgetParser(widgetToImport, availableMetrics, 'test-org').parse();
+    const result = new WidgetParser(
+      widgetToImport,
+      availableMetrics,
+      'test-org',
+      false
+    ).parse();
     const {report} = await result;
 
     expect(report.outcome).toEqual('error');
@@ -187,7 +209,7 @@ describe('WidgetParser', () => {
       requests: mockRequests(['sum:sentry.foo.bar{not-a-tag:bar} by {baz}']),
     });
 
-    const result = new WidgetParser(widget, availableMetrics, 'test-org').parse();
+    const result = new WidgetParser(widget, availableMetrics, 'test-org', false).parse();
     const {report} = await result;
 
     expect(report.outcome).toEqual('warning');
@@ -201,7 +223,7 @@ describe('WidgetParser', () => {
       requests: mockRequests(['sum:sentry.foo.bar{foo:bar*} by {baz}']),
     });
 
-    const result = new WidgetParser(widget, availableMetrics, 'test-org').parse();
+    const result = new WidgetParser(widget, availableMetrics, 'test-org', false).parse();
     const {report} = await result;
 
     expect(report.outcome).toEqual('warning');
@@ -239,7 +261,11 @@ describe('parseDashboard', () => {
       widgets: [mockWidget(), mockWidget()],
     } as ImportDashboard;
 
-    const result = await parseDashboard(dashboard, availableMetrics, 'test-org');
+    const result = await parseDashboard(
+      dashboard,
+      availableMetrics,
+      OrganizationFixture({slug: 'test-org'})
+    );
     const {report, widgets} = result;
 
     expect(report.length).toEqual(2);
@@ -263,7 +289,11 @@ describe('parseDashboard', () => {
       ],
     } as ImportDashboard;
 
-    const result = await parseDashboard(dashboard, availableMetrics, 'test-org');
+    const result = await parseDashboard(
+      dashboard,
+      availableMetrics,
+      OrganizationFixture({slug: 'test-org'})
+    );
     const {report, widgets} = result;
 
     expect(report.length).toEqual(2);
@@ -293,7 +323,11 @@ describe('parseDashboard', () => {
       ],
     } as ImportDashboard;
 
-    const result = await parseDashboard(dashboard, availableMetrics, 'test-org');
+    const result = await parseDashboard(
+      dashboard,
+      availableMetrics,
+      OrganizationFixture({slug: 'test-org'})
+    );
 
     const {report, widgets} = result;
     expect(report.length).toEqual(1);
@@ -301,8 +335,8 @@ describe('parseDashboard', () => {
 
     const queries = widgets[0].queries;
     expect(queries.length).toEqual(5);
-    expect(queries[2].aggregates[0]).toEqual('equation|2 * $B');
-    expect(queries[3].aggregates[0]).toEqual('equation|$A + $B');
-    expect(queries[4].aggregates[0]).toEqual('equation|($B + $B) - $A');
+    expect(queries[2].aggregates[0]).toEqual('equation|2 * $b');
+    expect(queries[3].aggregates[0]).toEqual('equation|$a + $b');
+    expect(queries[4].aggregates[0]).toEqual('equation|($b + $b) - $a');
   });
 });

+ 15 - 4
static/app/utils/metrics/dashboardImport.tsx

@@ -1,7 +1,9 @@
 import {Client} from 'sentry/api';
 import {getQuerySymbol} from 'sentry/components/metrics/querySymbol';
+import type {Organization} from 'sentry/types';
 import type {MetricMeta, MRI} from 'sentry/types/metrics';
 import {convertToDashboardWidget} from 'sentry/utils/metrics/dashboard';
+import {hasMetricsNewInputs} from 'sentry/utils/metrics/features';
 import type {MetricsQuery} from 'sentry/utils/metrics/types';
 import {MetricDisplayType} from 'sentry/utils/metrics/types';
 import type {Widget} from 'sentry/views/dashboards/types';
@@ -64,8 +66,9 @@ export type ParseResult = {
 export async function parseDashboard(
   dashboard: ImportDashboard,
   availableMetrics: MetricMeta[],
-  orgSlug: string
+  organization: Organization
 ): Promise<ParseResult> {
+  const metricsNewInputs = hasMetricsNewInputs(organization);
   const {widgets = []} = dashboard;
 
   const flatWidgets = widgets.flatMap(widget => {
@@ -78,7 +81,12 @@ export async function parseDashboard(
 
   const results = await Promise.all(
     flatWidgets.map(widget => {
-      const parser = new WidgetParser(widget, availableMetrics, orgSlug);
+      const parser = new WidgetParser(
+        widget,
+        availableMetrics,
+        organization.slug,
+        metricsNewInputs
+      );
       return parser.parse();
     })
   );
@@ -113,15 +121,18 @@ export class WidgetParser {
   private importedWidget: ImportWidget;
   private availableMetrics: MetricMeta[];
   private orgSlug: string;
+  private metricsNewInputs: boolean;
 
   constructor(
     importedWidget: ImportWidget,
     availableMetrics: MetricMeta[],
-    orgSlug: string
+    orgSlug: string,
+    metricsNewInputs: boolean
   ) {
     this.importedWidget = importedWidget;
     this.availableMetrics = availableMetrics;
     this.orgSlug = orgSlug;
+    this.metricsNewInputs = metricsNewInputs;
   }
 
   // Parsing functions
@@ -241,7 +252,7 @@ export class WidgetParser {
   private parseEquations(queries: any[], formulas: Formula[]) {
     const queryNames = queries.map(q => q.name);
     const queryNameMap = queries.reduce((acc, query, index) => {
-      acc[query.name] = getQuerySymbol(index);
+      acc[query.name] = getQuerySymbol(index, this.metricsNewInputs);
       return acc;
     }, {});
 

+ 1 - 4
static/app/utils/metrics/useMetricsQuery.tsx

@@ -203,10 +203,7 @@ export function useMetricsQuery(
       queries
         .map(query => {
           if (isMetricFormula(query)) {
-            return {
-              ...query,
-              formula: query.formula.toUpperCase(),
-            };
+            return query;
           }
           if (!isVirtualMetric(query)) {
             return query;

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