Skip to content

Commit 56cca50

Browse files
committed
ref(dashboards): extract useDashboardChartInterval hook
Consolidate the three dashboard useChartInterval call sites into a single wrapper hook that hardcodes USE_SECOND_BIGGEST strategy. This prevents detail.tsx, widgetPreview.tsx, and filtersBar.tsx from going out of sync. Also fixes filtersBar.tsx which was previously using the default USE_SMALLEST strategy instead of USE_SECOND_BIGGEST.
1 parent e9e32d0 commit 56cca50

File tree

5 files changed

+42
-36
lines changed

5 files changed

+42
-36
lines changed

static/app/views/dashboards/dashboard.spec.tsx

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@ import {resetMockDate, setMockDate} from 'sentry-test/utils';
1111
import {PageFiltersStore} from 'sentry/components/pageFilters/store';
1212
import {MemberListStore} from 'sentry/stores/memberListStore';
1313
import {MEPSettingProvider} from 'sentry/utils/performance/contexts/metricsEnhancedSetting';
14-
import {useChartInterval} from 'sentry/utils/useChartInterval';
1514
import {useLocation} from 'sentry/utils/useLocation';
1615
import {Dashboard} from 'sentry/views/dashboards/dashboard';
1716
import {FiltersBar} from 'sentry/views/dashboards/filtersBar';
17+
import {useDashboardChartInterval} from 'sentry/views/dashboards/hooks/useDashboardChartInterval';
1818
import type {
1919
DashboardDetails,
2020
DashboardFilters,
@@ -499,7 +499,7 @@ describe('Dashboards > Dashboard', () => {
499499
dashboard?: DashboardDetails;
500500
} = {}) {
501501
const location = useLocation();
502-
const [widgetInterval] = useChartInterval();
502+
const [widgetInterval] = useDashboardChartInterval();
503503
return (
504504
<MEPSettingProvider forceTransactions={false}>
505505
<FiltersBar
@@ -541,12 +541,12 @@ describe('Dashboards > Dashboard', () => {
541541
});
542542

543543
describe('no interval set in URL', () => {
544-
it('defaults to the smallest valid interval for the dashboard period', async () => {
545-
const fiveMinuteMock = MockApiClient.addMockResponse({
544+
it('defaults to the second-biggest valid interval for the dashboard period', async () => {
545+
const tenMinuteMock = MockApiClient.addMockResponse({
546546
url: '/organizations/org-slug/events-stats/',
547547
method: 'GET',
548548
body: [],
549-
match: [MockApiClient.matchQuery({interval: '5m'})],
549+
match: [MockApiClient.matchQuery({interval: '10m'})],
550550
});
551551
const thirtyMinuteMock = MockApiClient.addMockResponse({
552552
url: '/organizations/org-slug/events-stats/',
@@ -555,21 +555,21 @@ describe('Dashboards > Dashboard', () => {
555555
match: [MockApiClient.matchQuery({interval: '30m'})],
556556
});
557557

558-
// No interval in the URL — the 5m default is derived purely from the
559-
// dashboard's saved 24h period via PageFiltersStore → useChartInterval.
558+
// No interval in the URL — the 10m default (second-biggest of [5m, 10m, 30m])
559+
// is derived from the dashboard's saved 24h period via useDashboardChartInterval.
560560
const {router} = render(<DashboardWithIntervalSelector />, {
561561
organization: orgWithFlag,
562562
initialRouterConfig: {location: {pathname: '/'}},
563563
});
564564

565565
await screen.findByText('Test Spans Widget');
566-
await waitFor(() => expect(fiveMinuteMock).toHaveBeenCalled());
566+
await waitFor(() => expect(tenMinuteMock).toHaveBeenCalled());
567567
expect(thirtyMinuteMock).not.toHaveBeenCalled();
568568

569569
// Click the interval selector and choose '30 minutes'. FiltersBar writes
570570
// interval=30m to the URL, DashboardWithIntervalSelector re-renders with
571571
// the new widgetInterval, and the widget re-fetches with the new interval.
572-
await userEvent.click(screen.getByRole('button', {name: '5 minutes'}));
572+
await userEvent.click(screen.getByRole('button', {name: '10 minutes'}));
573573
await userEvent.click(screen.getByRole('option', {name: '30 minutes'}));
574574

575575
await waitFor(() => expect(thirtyMinuteMock).toHaveBeenCalled());
@@ -632,11 +632,11 @@ describe('Dashboards > Dashboard', () => {
632632
});
633633

634634
it('ignores the URL interval and falls back to the period default', async () => {
635-
const threeHourMock = MockApiClient.addMockResponse({
635+
const sixHourMock = MockApiClient.addMockResponse({
636636
url: '/organizations/org-slug/events-stats/',
637637
method: 'GET',
638638
body: [],
639-
match: [MockApiClient.matchQuery({interval: '3h'})],
639+
match: [MockApiClient.matchQuery({interval: '6h'})],
640640
});
641641
const fiveMinuteMock = MockApiClient.addMockResponse({
642642
url: '/organizations/org-slug/events-stats/',
@@ -653,11 +653,12 @@ describe('Dashboards > Dashboard', () => {
653653

654654
await screen.findByText('Test Spans Widget');
655655

656-
// The selector should show the period-derived default, not the invalid URL value.
657-
expect(screen.getByRole('button', {name: '3 hours'})).toBeInTheDocument();
656+
// The selector should show the period-derived default (6h = second-biggest
657+
// of [3h, 6h, 12h]), not the invalid URL value (5m).
658+
expect(screen.getByRole('button', {name: '6 hours'})).toBeInTheDocument();
658659

659660
// The widget should query with the valid default interval, not the URL value.
660-
await waitFor(() => expect(threeHourMock).toHaveBeenCalled());
661+
await waitFor(() => expect(sixHourMock).toHaveBeenCalled());
661662
expect(fiveMinuteMock).not.toHaveBeenCalled();
662663
});
663664
});
@@ -681,11 +682,11 @@ describe('Dashboards > Dashboard', () => {
681682
it('ignores the URL interval and falls back to the period default', async () => {
682683
// Uses the /sessions/ endpoint (session.status in columns → useSessionAPI=true),
683684
// which surfaces the "intervals too granular" error in production.
684-
const threeHourMock = MockApiClient.addMockResponse({
685+
const sixHourMock = MockApiClient.addMockResponse({
685686
url: '/organizations/org-slug/sessions/',
686687
method: 'GET',
687688
body: emptySessionsBody,
688-
match: [MockApiClient.matchQuery({interval: '3h'})],
689+
match: [MockApiClient.matchQuery({interval: '6h'})],
689690
});
690691
const fiveMinuteMock = MockApiClient.addMockResponse({
691692
url: '/organizations/org-slug/sessions/',
@@ -701,13 +702,13 @@ describe('Dashboards > Dashboard', () => {
701702

702703
await screen.findByText('Test Releases Widget');
703704

704-
// The selector should show the period-derived default (3h), not the
705-
// invalid URL value (5m).
706-
expect(screen.getByRole('button', {name: '3 hours'})).toBeInTheDocument();
705+
// The selector should show the period-derived default (6h = second-biggest
706+
// of [3h, 6h, 12h]), not the invalid URL value (5m).
707+
expect(screen.getByRole('button', {name: '6 hours'})).toBeInTheDocument();
707708

708709
// The widget should query the sessions endpoint with the valid default
709710
// interval, not the 5m value from the URL.
710-
await waitFor(() => expect(threeHourMock).toHaveBeenCalled());
711+
await waitFor(() => expect(sixHourMock).toHaveBeenCalled());
711712
expect(fiveMinuteMock).not.toHaveBeenCalled();
712713
});
713714
});

static/app/views/dashboards/detail.tsx

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,17 +53,14 @@ import {OnRouteLeave} from 'sentry/utils/reactRouter6Compat/onRouteLeave';
5353
import {scheduleMicroTask} from 'sentry/utils/scheduleMicroTask';
5454
import {normalizeUrl} from 'sentry/utils/url/normalizeUrl';
5555
import {useApi} from 'sentry/utils/useApi';
56-
import {
57-
ChartIntervalUnspecifiedStrategy,
58-
useChartInterval,
59-
} from 'sentry/utils/useChartInterval';
6056
import {useLocation} from 'sentry/utils/useLocation';
6157
import type {ReactRouter3Navigate} from 'sentry/utils/useNavigate';
6258
import {useNavigate} from 'sentry/utils/useNavigate';
6359
import {useOrganization} from 'sentry/utils/useOrganization';
6460
import {useParams} from 'sentry/utils/useParams';
6561
import {useProjects} from 'sentry/utils/useProjects';
6662
import {useRouter} from 'sentry/utils/useRouter';
63+
import {useDashboardChartInterval} from 'sentry/views/dashboards/hooks/useDashboardChartInterval';
6764
import {
6865
cloneDashboard,
6966
getCurrentPageFilters,
@@ -1465,9 +1462,7 @@ export function DashboardDetailWithInjectedProps(
14651462
const location = useLocation();
14661463
const params = useParams<RouteParams>();
14671464
const router = useRouter();
1468-
const [chartInterval] = useChartInterval({
1469-
unspecifiedStrategy: ChartIntervalUnspecifiedStrategy.USE_SECOND_BIGGEST,
1470-
});
1465+
const [chartInterval] = useDashboardChartInterval();
14711466
const queryClient = useQueryClient();
14721467
// Always use the validated chart interval so the UI dropdown and widget
14731468
// requests stay in sync. chartInterval is validated against the current page

static/app/views/dashboards/filtersBar.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,14 @@ import type {User} from 'sentry/types/user';
2525
import {defined} from 'sentry/utils';
2626
import {trackAnalytics} from 'sentry/utils/analytics';
2727
import {ToggleOnDemand} from 'sentry/utils/performance/contexts/onDemandControl';
28-
import {useChartInterval} from 'sentry/utils/useChartInterval';
2928
import {useMaxPickableDays} from 'sentry/utils/useMaxPickableDays';
3029
import {useOrganization} from 'sentry/utils/useOrganization';
3130
import {useUser} from 'sentry/utils/useUser';
3231
import {useUserTeams} from 'sentry/utils/useUserTeams';
3332
import {AddFilter} from 'sentry/views/dashboards/globalFilter/addFilter';
3433
import {GenericFilterSelector} from 'sentry/views/dashboards/globalFilter/genericFilterSelector';
3534
import {globalFilterKeysAreEqual} from 'sentry/views/dashboards/globalFilter/utils';
35+
import {useDashboardChartInterval} from 'sentry/views/dashboards/hooks/useDashboardChartInterval';
3636
import {useDatasetSearchBarData} from 'sentry/views/dashboards/hooks/useDatasetSearchBarData';
3737
import {useInvalidateStarredDashboards} from 'sentry/views/dashboards/hooks/useInvalidateStarredDashboards';
3838
import {getDashboardFiltersFromURL} from 'sentry/views/dashboards/utils';
@@ -234,7 +234,7 @@ export function FiltersBar({
234234
const hasIntervalSelection = organization.features.includes(
235235
'dashboards-interval-selection'
236236
);
237-
const [interval, setInterval, intervalOptions] = useChartInterval();
237+
const [interval, setInterval, intervalOptions] = useDashboardChartInterval();
238238

239239
return (
240240
<Wrapper>
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import {
2+
ChartIntervalUnspecifiedStrategy,
3+
useChartInterval,
4+
} from 'sentry/utils/useChartInterval';
5+
6+
/**
7+
* Wrapper around `useChartInterval` that uses the `USE_SECOND_BIGGEST`
8+
* strategy for dashboards. This keeps the dashboard detail page, widget
9+
* builder preview, and filters bar in sync.
10+
*/
11+
export function useDashboardChartInterval() {
12+
return useChartInterval({
13+
unspecifiedStrategy: ChartIntervalUnspecifiedStrategy.USE_SECOND_BIGGEST,
14+
});
15+
}

static/app/views/dashboards/widgetBuilder/components/widgetPreview.tsx

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,10 @@ import {useState} from 'react';
33
import {usePageFilters} from 'sentry/components/pageFilters/usePageFilters';
44
import {dedupeArray} from 'sentry/utils/dedupeArray';
55
import type {Sort} from 'sentry/utils/discover/fields';
6-
import {
7-
ChartIntervalUnspecifiedStrategy,
8-
useChartInterval,
9-
} from 'sentry/utils/useChartInterval';
106
import {useLocation} from 'sentry/utils/useLocation';
117
import {useNavigate} from 'sentry/utils/useNavigate';
128
import {useOrganization} from 'sentry/utils/useOrganization';
9+
import {useDashboardChartInterval} from 'sentry/views/dashboards/hooks/useDashboardChartInterval';
1310
import {
1411
DisplayType,
1512
WidgetType,
@@ -47,9 +44,7 @@ export function WidgetPreview({
4744
const location = useLocation();
4845
const navigate = useNavigate();
4946
const pageFilters = usePageFilters();
50-
const [chartInterval] = useChartInterval({
51-
unspecifiedStrategy: ChartIntervalUnspecifiedStrategy.USE_SECOND_BIGGEST,
52-
});
47+
const [chartInterval] = useDashboardChartInterval();
5348

5449
const {state, dispatch} = useWidgetBuilderContext();
5550
const [tableWidths, setTableWidths] = useState<number[]>();

0 commit comments

Comments
 (0)