Skip to content

feat(discover): Discover Bar Chart Display Mode #29485

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
edwardgou-sentry merged 6 commits into from
Oct 22, 2021
Merged

feat(discover): Discover Bar Chart Display Mode #29485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0755b32
Disable other y axis options when in a display mode that doesnt support
edwardgou-sentry Oct 20, 2021
de10404
hide checkbox
edwardgou-sentry Oct 21, 2021
fbc9ed0
Added barchart display mode to discover
edwardgou-sentry Oct 21, 2021
e1c929a
minigraph bar chart
edwardgou-sentry Oct 21, 2021
0504234
interval test
edwardgou-sentry Oct 21, 2021
808d0de
Merge branch 'master' into feat/discover-bar-chart
edwardgou-sentry Oct 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions static/app/utils/discover/types.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export enum DisplayModes {
DAILY = 'daily',
DAILYTOP5 = 'dailytop5',
WORLDMAP = 'worldmap',
BAR = 'bar',
}

export const TOP_EVENT_MODES: string[] = [DisplayModes.TOP5, DisplayModes.DAILYTOP5];
Expand All @@ -21,6 +22,7 @@ export const DISPLAY_MODE_OPTIONS: SelectValue<string>[] = [
{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 @@ -37,6 +39,7 @@ export const DISPLAY_MODE_FALLBACK_OPTIONS = {
[DisplayModes.DAILY]: DisplayModes.DEFAULT,
[DisplayModes.DAILYTOP5]: DisplayModes.DAILY,
[DisplayModes.WORLDMAP]: DisplayModes.DEFAULT,
[DisplayModes.BAR]: DisplayModes.DEFAULT,
};

// default list of yAxis options
Expand All @@ -49,4 +52,5 @@ export const MULTI_Y_AXIS_SUPPORTED_DISPLAY_MODES = [
DisplayModes.DEFAULT,
DisplayModes.DAILY,
DisplayModes.PREVIOUS,
DisplayModes.BAR,
];
18 changes: 12 additions & 6 deletions static/app/views/eventsV2/miniGraph.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ class MiniGraph extends React.Component<Props> {
const field = isTopEvents ? apiPayload.field : undefined;
const topEvents = isTopEvents ? TOP_N : undefined;
const orderby = isTopEvents ? decodeScalar(apiPayload.sort) : undefined;
const interval = isDaily ? '1d' : getInterval({start, end, period}, 'high');
const intervalFidelity = display === 'bar' ? 'low' : 'high';
const interval = isDaily ? '1d' : getInterval({start, end, period}, intervalFidelity);

return {
organization,
Expand All @@ -82,6 +83,7 @@ class MiniGraph extends React.Component<Props> {
showDaily: isDaily,
expired: eventView.expired,
name: eventView.name,
display,
};
}

Expand Down Expand Up @@ -131,6 +133,7 @@ class MiniGraph extends React.Component<Props> {
showDaily,
expired,
name,
display,
} = this.getRefreshProps(this.props);

return (
Expand Down Expand Up @@ -171,11 +174,14 @@ class MiniGraph extends React.Component<Props> {
}

const allSeries = timeseriesData ?? results ?? [];
const chartType = this.getChartType({
showDaily,
yAxis: Array.isArray(yAxis) ? yAxis[0] : yAxis,
timeseriesData: allSeries,
});
const chartType =
display === 'bar'
? display
: this.getChartType({
showDaily,
yAxis: Array.isArray(yAxis) ? yAxis[0] : yAxis,
timeseriesData: allSeries,
});
const data = allSeries.map(series => ({
...series,
lineStyle: {
Expand Down
22 changes: 18 additions & 4 deletions static/app/views/eventsV2/resultsChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import isEqual from 'lodash/isEqual';

import {Client} from 'app/api';
import AreaChart from 'app/components/charts/areaChart';
import BarChart from 'app/components/charts/barChart';
import EventsChart from 'app/components/charts/eventsChart';
import {getInterval} from 'app/components/charts/utils';
import WorldMapChart from 'app/components/charts/worldMapChart';
import {getParams} from 'app/components/organizations/globalSelectionHeader/getParams';
import {Panel} from 'app/components/panels';
Expand Down Expand Up @@ -83,9 +85,23 @@ class ResultsChart extends Component<ResultsChartProps> {
const chartComponent =
display === DisplayModes.WORLDMAP
? WorldMapChart
: display === DisplayModes.BAR
? BarChart
: hasConnectDiscoverAndDashboards && yAxisValue.length > 1 && !isDaily
? AreaChart
: undefined;
const interval =
display === DisplayModes.BAR
? getInterval(
{
start,
end,
period: globalSelection.datetime.period,
utc: utc === 'true',
},
'low'
)
: eventView.interval;

return (
<Fragment>
Expand All @@ -106,7 +122,7 @@ class ResultsChart extends Component<ResultsChartProps> {
disablePrevious={!isPrevious}
disableReleases={!isPeriod}
field={isTopEvents ? apiPayload.field : undefined}
interval={eventView.interval}
interval={interval}
showDaily={isDaily}
topEvents={isTopEvents ? topEvents : undefined}
orderby={isTopEvents ? decodeScalar(apiPayload.sort) : undefined}
Expand Down Expand Up @@ -205,9 +221,7 @@ class ResultsChartContainer extends Component<ContainerProps> {
}
if (
yAxis.length > 1 &&
![DisplayModes.DEFAULT, DisplayModes.DAILY, DisplayModes.PREVIOUS].includes(
opt.value as DisplayModes
)
!MULTI_Y_AXIS_SUPPORTED_DISPLAY_MODES.includes(opt.value as DisplayModes)
) {
return {
...opt,
Expand Down
17 changes: 17 additions & 0 deletions tests/js/spec/views/eventsV2/miniGraph.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,21 @@ describe('EventsV2 > MiniGraph', function () {
const eventsRequestProps = wrapper.find('EventsRequest').props();
expect(eventsRequestProps.yAxis).toEqual(yAxis);
});

it('uses low fidelity interval for bar charts', async function () {
const yAxis = ['count()', 'failure_count()'];
eventView.display = 'bar';
const wrapper = mountWithTheme(
<MiniGraph
// @ts-expect-error
location={location}
eventView={eventView}
organization={organization}
yAxis={yAxis}
/>,
initialData.routerContext
);
const eventsRequestProps = wrapper.find('EventsRequest').props();
expect(eventsRequestProps.interval).toEqual('12h');
});
});
11 changes: 8 additions & 3 deletions tests/js/spec/views/eventsV2/resultsChart.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('EventsV2 > ResultsChart', function () {
eventView = EventView.fromSavedQueryOrLocation(undefined, location);
});

it('only allows default, daily, and previous period display modes when multiple y axis are selected', async function () {
it('only allows default, daily, previous period, and bar display modes when multiple y axis are selected', async function () {
const wrapper = mountWithTheme(
<ResultsChart
// @ts-expect-error
Expand All @@ -54,7 +54,12 @@ describe('EventsV2 > ResultsChart', function () {
const displayOptions = wrapper.find('ChartFooter').props().displayOptions;
displayOptions.forEach(({value, disabled}) => {
if (
![DisplayModes.DEFAULT, DisplayModes.DAILY, DisplayModes.PREVIOUS].includes(value)
![
DisplayModes.DEFAULT,
DisplayModes.DAILY,
DisplayModes.PREVIOUS,
DisplayModes.BAR,
].includes(value)
) {
expect(disabled).toBe(true);
}
Expand Down Expand Up @@ -83,7 +88,7 @@ describe('EventsV2 > ResultsChart', function () {
);
});

it('disables other y-axis options when not in default, daily, or previous period display mode', async function () {
it('disables other y-axis options when not in default, daily, previous period, or bar display mode', async function () {
eventView.display = DisplayModes.WORLDMAP;
const wrapper = mountWithTheme(
<ResultsChart
Expand Down