From a09c35d0c834bf940c26b494c865b9cb0556f8c7 Mon Sep 17 00:00:00 2001 From: Abdkhan14 <60121741+Abdkhan14@users.noreply.github.com> Date: Tue, 25 Jul 2023 14:03:49 -0400 Subject: [PATCH] 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: Screenshot 2023-07-19 at 3 59 54 PM - 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 --- .../components/charts/eventsChart.spec.tsx | 67 ------------------- static/app/components/charts/eventsChart.tsx | 29 -------- ...dashboardWidgetQuerySelectorModal.spec.tsx | 11 --- static/app/utils/discover/types.tsx | 3 - static/app/views/dashboards/utils.tsx | 3 - .../dashboards/widgetCard/index.spec.tsx | 36 ---------- static/app/views/discover/miniGraph.spec.tsx | 39 +---------- static/app/views/discover/miniGraph.tsx | 51 +------------- .../app/views/discover/resultsChart.spec.tsx | 38 ----------- static/app/views/discover/resultsChart.tsx | 29 ++------ .../app/views/discover/savedQuery/utils.tsx | 2 - 11 files changed, 7 insertions(+), 301 deletions(-) delete mode 100644 static/app/components/charts/eventsChart.spec.tsx diff --git a/static/app/components/charts/eventsChart.spec.tsx b/static/app/components/charts/eventsChart.spec.tsx deleted file mode 100644 index 8f6e8eb45ee014..00000000000000 --- a/static/app/components/charts/eventsChart.spec.tsx +++ /dev/null @@ -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(() =>
); -}); -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( - , - {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() - ); - }); -}); diff --git a/static/app/components/charts/eventsChart.tsx b/static/app/components/charts/eventsChart.tsx index f16e5310828e56..e70a986c6ce500 100644 --- a/static/app/components/charts/eventsChart.tsx +++ b/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 { {...props} > {zoomRenderProps => { - if (chartComponent === WorldMapChart) { - return ( - - {({errored, loading, reloading, tableData}) => - chartImplementation({ - errored, - loading, - reloading, - zoomRenderProps, - tableData, - }) - } - - ); - } return ( 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' - ); - }); }); diff --git a/static/app/utils/discover/types.tsx b/static/app/utils/discover/types.tsx index fbc4de8fe0f1e0..c0b66b7d2535d5 100644 --- a/static/app/utils/discover/types.tsx +++ b/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[] = [ {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, }; diff --git a/static/app/views/dashboards/utils.tsx b/static/app/views/dashboards/utils.tsx index 950e91f256f09c..18769e5a541c46 100644 --- a/static/app/views/dashboards/utils.tsx +++ b/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; diff --git a/static/app/views/dashboards/widgetCard/index.spec.tsx b/static/app/views/dashboards/widgetCard/index.spec.tsx index 5773bd144d737a..d538ad5a6a12af 100644 --- a/static/app/views/dashboards/widgetCard/index.spec.tsx +++ b/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( - 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( MiniGraph', function () { expect.anything() ); }); - - it('renders WorldMapChart', async function () { - const yAxis = ['count()', 'failure_count()']; - eventView.display = 'worldmap'; - - jest.spyOn(worldMaps, 'WorldMapChart'); - - render( - , - {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() - ) - ); - }); }); diff --git a/static/app/views/discover/miniGraph.tsx b/static/app/views/discover/miniGraph.tsx index 286f564c28ee93..4dce207e971b8a 100644 --- a/static/app/views/discover/miniGraph.tsx +++ b/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 { display, } = this.getRefreshProps(this.props); - if (display === DisplayModes.WORLDMAP) { - return ( - - {({errored, loading, tableData}) => { - if (errored) { - return ( - - - - ); - } - if (loading) { - return ( - - - - ); - } - const {data, title} = processTableResults(tableData); - const chartOptions = { - height: 100, - series: [ - { - seriesName: title, - data, - }, - ], - fromDiscoverQueryList: true, - }; - - return ; - }} - - ); - } return ( 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( - 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(); - }); }); diff --git a/static/app/views/discover/resultsChart.tsx b/static/app/views/discover/resultsChart.tsx index 4f9853d5fd416c..e3dda771d6a7a2 100644 --- a/static/app/views/discover/resultsChart.tsx +++ b/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 { : 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 { 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 { 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, diff --git a/static/app/views/discover/savedQuery/utils.tsx b/static/app/views/discover/savedQuery/utils.tsx index ace55669395e2c..cef2e01d5951cb 100644 --- a/static/app/views/discover/savedQuery/utils.tsx +++ b/static/app/views/discover/savedQuery/utils.tsx @@ -237,8 +237,6 @@ export function displayModeToDisplayType(displayMode: DisplayModes): DisplayType switch (displayMode) { case DisplayModes.BAR: return DisplayType.BAR; - case DisplayModes.WORLDMAP: - return DisplayType.WORLD_MAP; case DisplayModes.TOP5: return DisplayType.TOP_N; default: