Skip to content

Commit

Permalink
feat(dnd-worldmap-removal) Removed worldmap display option from disco…
Browse files Browse the repository at this point in the history
…ver chart. (#53185)

For issue: #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](#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>
  • Loading branch information
Abdkhan14 and Abdullah Khan authored Jul 25, 2023
1 parent b9b5c8a commit a09c35d
Show file tree
Hide file tree
Showing 11 changed files with 7 additions and 301 deletions.
67 changes: 0 additions & 67 deletions static/app/components/charts/eventsChart.spec.tsx

This file was deleted.

29 changes: 0 additions & 29 deletions static/app/components/charts/eventsChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
);
});
});
3 changes: 0 additions & 3 deletions static/app/utils/discover/types.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ export enum DisplayModes {
TOP5 = 'top5',
DAILY = 'daily',
DAILYTOP5 = 'dailytop5',
WORLDMAP = 'worldmap',
BAR = 'bar',
}

Expand Down Expand Up @@ -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')},
];

Expand All @@ -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,
};

Expand Down
3 changes: 0 additions & 3 deletions static/app/views/dashboards/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
36 changes: 0 additions & 36 deletions static/app/views/dashboards/widgetCard/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 1 addition & 38 deletions static/app/views/discover/miniGraph.spec.tsx
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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()
)
);
});
});
51 changes: 1 addition & 50 deletions static/app/views/discover/miniGraph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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}
Expand Down
38 changes: 0 additions & 38 deletions static/app/views/discover/resultsChart.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
Loading

0 comments on commit a09c35d

Please sign in to comment.