Просмотр исходного кода

feat(dnd-worldmap-removal) Removed worldmap display option from discover chart. (#53185)

For issue: https://github.com/getsentry/sentry/issues/51315
Steps outlined:
[here](https://www.notion.so/sentry/Removing-World-Map-display-from-Discover-and-Dashboards-e49eb198fe294ad0a1b4c47b9c3619c0?pvs=4#43e2c318d1eb4f93987718b82307f56f).

- Won't be merging until, data-migration
[PR](https://github.com/getsentry/sentry/pull/53109) for converting
world map widgets to table widgets is deployed.
- Removes the World map display mode from discover chart footer:
<img width="1104" alt="Screenshot 2023-07-19 at 3 59 54 PM"
src="https://github.com/getsentry/sentry/assets/60121741/80ca9e8e-aeb4-45ab-92a4-bda951b426f5">

- Should not effect the loading of existing discover saved queries,
homepages and previews with world map display mode:
- - Falls back to AreaChart instead of WorldMapChart:
[code](https://github.com/getsentry/sentry/blob/4bd6934f1b3f947043ef93b1dd35a11b1e9927cf/static/app/components/charts/eventsChart.tsx#L140-L163)
- - Falls back to Total Period display mode instead:
[code](https://github.com/getsentry/sentry/blob/9b816eae27f4a29eedad50e5ed8af86188badec1/static/app/utils/discover/eventView.tsx#L1403-L1426)
- Removed tests and blocks of code using: `DisplayMode.WORLDMAP`.
- Added unused components like `WorldMapChart` and `EventsGeoRequest` to
clean up list for removal.

---------

Co-authored-by: Abdullah Khan <abdullahkhan@PG9Y57YDXQ.local>
Abdkhan14 1 год назад
Родитель
Сommit
a09c35d0c8

+ 0 - 67
static/app/components/charts/eventsChart.spec.tsx

@@ -1,67 +0,0 @@
-import {initializeOrg} from 'sentry-test/initializeOrg';
-import {render, screen} from 'sentry-test/reactTestingLibrary';
-
-import BaseChart from 'sentry/components/charts/baseChart';
-import EventsChart from 'sentry/components/charts/eventsChart';
-import {WorldMapChart} from 'sentry/components/charts/worldMapChart';
-
-jest.mock('sentry/components/charts/baseChart', () => {
-  return jest.fn().mockImplementation(() => <div data-test-id="chart" />);
-});
-jest.mock(
-  'sentry/components/charts/eventsGeoRequest',
-  () =>
-    ({children}) =>
-      children({
-        errored: false,
-        loading: false,
-        reloading: false,
-        tableData: [],
-      })
-);
-
-describe('EventsChart', function () {
-  const {router, routerContext, organization} = initializeOrg();
-
-  afterEach(() => {
-    MockApiClient.clearMockResponses();
-  });
-
-  function renderChart(props) {
-    return render(
-      <EventsChart
-        api={new MockApiClient()}
-        location={{query: {}}}
-        organization={organization}
-        project={[]}
-        environment={[]}
-        period="14d"
-        start={null}
-        end={null}
-        utc={false}
-        router={router}
-        {...props}
-      />,
-      {context: routerContext}
-    );
-  }
-
-  it('renders with World Map when given WorldMapChart chartComponent', async function () {
-    MockApiClient.addMockResponse({
-      url: '/organizations/org-slug/releases/stats/',
-      body: [],
-    });
-    renderChart({chartComponent: WorldMapChart, yAxis: ['count()']});
-    expect(await screen.findByTestId('chart')).toBeInTheDocument();
-    expect(BaseChart).toHaveBeenCalledWith(
-      expect.objectContaining({
-        series: [
-          expect.objectContaining({
-            map: 'sentryWorld',
-          }),
-        ],
-      }),
-      expect.anything()
-    );
-  });
-});

+ 0 - 29
static/app/components/charts/eventsChart.tsx

@@ -47,7 +47,6 @@ import {
 import {DiscoverDatasets} from 'sentry/utils/discover/types';
 import {decodeList} from 'sentry/utils/queryString';
 
-import EventsGeoRequest from './eventsGeoRequest';
 import EventsRequest from './eventsRequest';
 
 type ChartComponent =
@@ -671,34 +670,6 @@ class EventsChart extends Component<EventsChartProps> {
         {...props}
       >
         {zoomRenderProps => {
-          if (chartComponent === WorldMapChart) {
-            return (
-              <EventsGeoRequest
-                api={api}
-                organization={organization}
-                yAxis={yAxis}
-                query={query}
-                orderby={orderby}
-                projects={projects}
-                period={period}
-                start={start}
-                end={end}
-                environments={environments}
-                referrer={props.referrer}
-                dataset={dataset}
-              >
-                {({errored, loading, reloading, tableData}) =>
-                  chartImplementation({
-                    errored,
-                    loading,
-                    reloading,
-                    zoomRenderProps,
-                    tableData,
-                  })
-                }
-              </EventsGeoRequest>
-            );
-          }
           return (
             <EventsRequest
               {...props}

+ 0 - 11
static/app/components/modals/dashboardWidgetQuerySelectorModal.spec.tsx

@@ -125,15 +125,4 @@ describe('Modals -> AddDashboardWidgetModal', function () {
       '/organizations/org-slug/discover/results/?field=count%28%29&field=failure_count%28%29&name=Test%20Widget&query=title%3A%2Forganizations%2F%3AorgId%2Fperformance%2Fsummary%2F&statsPeriod=14d&yAxis=count%28%29&yAxis=failure_count%28%29'
     );
   });
-
-  it('links user to the query in discover with additional field when a world map query is selected from the modal', function () {
-    mockWidget.queries[0].fields = ['count()'];
-    mockWidget.queries[0].aggregates = ['count()'];
-    mockWidget.displayType = DisplayType.WORLD_MAP;
-    renderModal({initialData, widget: mockWidget});
-    expect(screen.getByRole('link')).toHaveAttribute(
-      'href',
-      '/organizations/org-slug/discover/results/?display=worldmap&field=geo.country_code&field=count%28%29&name=Test%20Widget&query=title%3A%2Forganizations%2F%3AorgId%2Fperformance%2Fsummary%2F%20has%3Ageo.country_code&statsPeriod=14d&yAxis=count%28%29'
-    );
-  });
 });

+ 0 - 3
static/app/utils/discover/types.tsx

@@ -9,7 +9,6 @@ export enum DisplayModes {
   TOP5 = 'top5',
   DAILY = 'daily',
   DAILYTOP5 = 'dailytop5',
-  WORLDMAP = 'worldmap',
   BAR = 'bar',
 }
 
@@ -38,7 +37,6 @@ export const DISPLAY_MODE_OPTIONS: SelectValue<string>[] = [
   {value: DisplayModes.TOP5, label: t('Top 5 Period')},
   {value: DisplayModes.DAILY, label: t('Total Daily')},
   {value: DisplayModes.DAILYTOP5, label: t('Top 5 Daily')},
-  {value: DisplayModes.WORLDMAP, label: t('World Map')},
   {value: DisplayModes.BAR, label: t('Bar Chart')},
 ];
 
@@ -55,7 +53,6 @@ export const DISPLAY_MODE_FALLBACK_OPTIONS = {
   [DisplayModes.TOP5]: DisplayModes.DEFAULT,
   [DisplayModes.DAILY]: DisplayModes.DEFAULT,
   [DisplayModes.DAILYTOP5]: DisplayModes.DAILY,
-  [DisplayModes.WORLDMAP]: DisplayModes.DEFAULT,
   [DisplayModes.BAR]: DisplayModes.DEFAULT,
 };
 

+ 0 - 3
static/app/views/dashboards/utils.tsx

@@ -242,9 +242,6 @@ export function getWidgetDiscoverUrl(
 
   // Visualization specific transforms
   switch (widget.displayType) {
-    case DisplayType.WORLD_MAP:
-      discoverLocation.query.display = DisplayModes.WORLDMAP;
-      break;
     case DisplayType.BAR:
       discoverLocation.query.display = DisplayModes.BAR;
       break;

+ 0 - 36
static/app/views/dashboards/widgetCard/index.spec.tsx

@@ -168,42 +168,6 @@ describe('Dashboards > WidgetCard', function () {
     expect(await screen.findByText('Valid widget description')).toBeInTheDocument();
   });
 
-  it('Opens in Discover with World Map', async function () {
-    renderWithProviders(
-      <WidgetCard
-        api={api}
-        organization={organization}
-        widget={{
-          ...multipleQueryWidget,
-          displayType: DisplayType.WORLD_MAP,
-          queries: [
-            {
-              ...multipleQueryWidget.queries[0],
-              fields: ['count()'],
-              aggregates: ['count()'],
-              columns: [],
-            },
-          ],
-        }}
-        selection={selection}
-        isEditing={false}
-        onDelete={() => undefined}
-        onEdit={() => undefined}
-        onDuplicate={() => undefined}
-        renderErrorMessage={() => undefined}
-        showContextMenu
-        widgetLimitReached={false}
-      />
-    );
-
-    await userEvent.click(await screen.findByLabelText('Widget actions'));
-    expect(screen.getByText('Open in Discover')).toBeInTheDocument();
-    await userEvent.click(screen.getByText('Open in Discover'));
-    expect(router.push).toHaveBeenCalledWith(
-      '/organizations/org-slug/discover/results/?display=worldmap&environment=prod&field=geo.country_code&field=count%28%29&name=Errors&project=1&query=event.type%3Aerror%20has%3Ageo.country_code&statsPeriod=14d&yAxis=count%28%29'
-    );
-  });
-
   it('Opens in Discover with prepended fields pulled from equations', async function () {
     renderWithProviders(
       <WidgetCard

+ 1 - 38
static/app/views/discover/miniGraph.spec.tsx

@@ -1,8 +1,7 @@
 import {initializeOrg} from 'sentry-test/initializeOrg';
-import {render, waitFor} from 'sentry-test/reactTestingLibrary';
+import {render} from 'sentry-test/reactTestingLibrary';
 
 import * as eventRequest from 'sentry/components/charts/eventsRequest';
-import * as worldMaps from 'sentry/components/charts/worldMapChart';
 import EventView from 'sentry/utils/discover/eventView';
 import MiniGraph from 'sentry/views/discover/miniGraph';
 
@@ -104,40 +103,4 @@ describe('Discover > MiniGraph', function () {
       expect.anything()
     );
   });
-
-  it('renders WorldMapChart', async function () {
-    const yAxis = ['count()', 'failure_count()'];
-    eventView.display = 'worldmap';
-
-    jest.spyOn(worldMaps, 'WorldMapChart');
-
-    render(
-      <MiniGraph
-        location={location}
-        eventView={eventView}
-        organization={organization}
-        yAxis={yAxis}
-      />,
-      {context: initialData.routerContext}
-    );
-
-    await waitFor(() =>
-      expect(worldMaps.WorldMapChart).toHaveBeenCalledWith(
-        {
-          height: 100,
-          fromDiscoverQueryList: true,
-          series: [
-            {
-              data: [
-                {name: 'PE', value: 9215},
-                {name: 'VI', value: 1},
-              ],
-              seriesName: 'Country',
-            },
-          ],
-        },
-        expect.anything()
-      )
-    );
-  });
 });

+ 1 - 50
static/app/views/discover/miniGraph.tsx

@@ -7,11 +7,9 @@ import isEqual from 'lodash/isEqual';
 import {Client} from 'sentry/api';
 import {AreaChart, AreaChartProps} from 'sentry/components/charts/areaChart';
 import {BarChart, BarChartProps} from 'sentry/components/charts/barChart';
-import EventsGeoRequest from 'sentry/components/charts/eventsGeoRequest';
 import EventsRequest from 'sentry/components/charts/eventsRequest';
 import {LineChart} from 'sentry/components/charts/lineChart';
-import {getInterval, processTableResults} from 'sentry/components/charts/utils';
-import {WorldMapChart} from 'sentry/components/charts/worldMapChart';
+import {getInterval} from 'sentry/components/charts/utils';
 import LoadingContainer from 'sentry/components/loading/loadingContainer';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import {IconWarning} from 'sentry/icons';
@@ -141,53 +139,6 @@ class MiniGraph extends Component<Props> {
       display,
     } = this.getRefreshProps(this.props);
 
-    if (display === DisplayModes.WORLDMAP) {
-      return (
-        <EventsGeoRequest
-          api={api}
-          organization={organization}
-          yAxis={yAxis}
-          query={query}
-          orderby={orderby}
-          projects={project as number[]}
-          period={period}
-          start={start}
-          end={end}
-          environments={environment as string[]}
-          referrer={referrer}
-        >
-          {({errored, loading, tableData}) => {
-            if (errored) {
-              return (
-                <StyledGraphContainer>
-                  <IconWarning color="gray300" size="md" />
-                </StyledGraphContainer>
-              );
-            }
-            if (loading) {
-              return (
-                <StyledGraphContainer>
-                  <LoadingIndicator mini />
-                </StyledGraphContainer>
-              );
-            }
-            const {data, title} = processTableResults(tableData);
-            const chartOptions = {
-              height: 100,
-              series: [
-                {
-                  seriesName: title,
-                  data,
-                },
-              ],
-              fromDiscoverQueryList: true,
-            };
-
-            return <WorldMapChart {...chartOptions} />;
-          }}
-        </EventsGeoRequest>
-      );
-    }
     return (
       <EventsRequest
         organization={organization}

+ 0 - 38
static/app/views/discover/resultsChart.spec.tsx

@@ -93,42 +93,4 @@ describe('Discover > ResultsChart', function () {
 
     expect(screen.getByText(/No Y-Axis selected/)).toBeInTheDocument();
   });
-
-  it('disables equation y-axis options when in World Map display mode', async function () {
-    eventView.display = DisplayModes.WORLDMAP;
-    eventView.fields = [
-      {field: 'count()'},
-      {field: 'count_unique(user)'},
-      {field: 'equation|count() + 2'},
-    ];
-
-    MockApiClient.addMockResponse({
-      url: `/organizations/${organization.slug}/events-geo/`,
-      body: [],
-    });
-
-    render(
-      <ResultsChart
-        router={TestStubs.router()}
-        organization={organization}
-        eventView={eventView}
-        location={location}
-        onAxisChange={() => undefined}
-        onDisplayChange={() => undefined}
-        onIntervalChange={() => undefined}
-        total={1}
-        confirmedQuery
-        yAxis={['count()']}
-        onTopEventsChange={() => {}}
-      />,
-      {context: initialData.routerContext}
-    );
-
-    await userEvent.click(await screen.findByText(/Y-Axis/));
-
-    expect(screen.getAllByRole('option')).toHaveLength(2);
-
-    expect(screen.getByRole('option', {name: 'count()'})).toBeEnabled();
-    expect(screen.getByRole('option', {name: 'count_unique(user)'})).toBeEnabled();
-  });
 });

+ 5 - 24
static/app/views/discover/resultsChart.tsx

@@ -9,7 +9,6 @@ import {AreaChart} from 'sentry/components/charts/areaChart';
 import {BarChart} from 'sentry/components/charts/barChart';
 import EventsChart from 'sentry/components/charts/eventsChart';
 import {getInterval, getPreviousSeriesName} from 'sentry/components/charts/utils';
-import {WorldMapChart} from 'sentry/components/charts/worldMapChart';
 import {normalizeDateTimeParams} from 'sentry/components/organizations/pageFilters/parse';
 import Panel from 'sentry/components/panels/panel';
 import Placeholder from 'sentry/components/placeholder';
@@ -20,11 +19,7 @@ import {CustomMeasurementCollection} from 'sentry/utils/customMeasurements/custo
 import {CustomMeasurementsContext} from 'sentry/utils/customMeasurements/customMeasurementsContext';
 import {getUtcToLocalDateObject} from 'sentry/utils/dates';
 import EventView from 'sentry/utils/discover/eventView';
-import {
-  getAggregateArg,
-  isEquation,
-  stripEquationPrefix,
-} from 'sentry/utils/discover/fields';
+import {getAggregateArg, stripEquationPrefix} from 'sentry/utils/discover/fields';
 import {
   DisplayModes,
   MULTI_Y_AXIS_SUPPORTED_DISPLAY_MODES,
@@ -104,9 +99,7 @@ class ResultsChart extends Component<ResultsChartProps> {
         : null
       : null;
     const chartComponent =
-      display === DisplayModes.WORLDMAP
-        ? WorldMapChart
-        : display === DisplayModes.BAR
+      display === DisplayModes.BAR
         ? BarChart
         : customPerformanceMetricFieldType === 'size' && isTopEvents
         ? AreaChart
@@ -193,12 +186,12 @@ type ContainerState = {
 
 class ResultsChartContainer extends Component<ContainerProps, ContainerState> {
   state: ContainerState = {
-    yAxisOptions: this.getYAxisOptions(this.props.eventView),
+    yAxisOptions: this.props.eventView.getYAxisOptions(),
   };
 
   UNSAFE_componentWillReceiveProps(nextProps) {
-    const yAxisOptions = this.getYAxisOptions(this.props.eventView);
-    const nextYAxisOptions = this.getYAxisOptions(nextProps.eventView);
+    const yAxisOptions = this.props.eventView.getYAxisOptions();
+    const nextYAxisOptions = nextProps.eventView.getYAxisOptions();
 
     if (!valueIsEqual(yAxisOptions, nextYAxisOptions, true)) {
       this.setState({yAxisOptions: nextYAxisOptions});
@@ -219,18 +212,6 @@ class ResultsChartContainer extends Component<ContainerProps, ContainerState> {
     return !isEqual(restProps, restNextProps);
   }
 
-  getYAxisOptions(eventView) {
-    const yAxisOptions = eventView.getYAxisOptions();
-
-    // Equations on World Map isn't supported on the events-geo endpoint
-    // Disabling equations as an option to prevent erroring out
-    if (eventView.getDisplayMode() === DisplayModes.WORLDMAP) {
-      return yAxisOptions.filter(({value}) => !isEquation(value));
-    }
-
-    return yAxisOptions;
-  }
-
   render() {
     const {
       api,

Некоторые файлы не были показаны из-за большого количества измененных файлов