From e3069cab577b777cdd8f955c0d3ee531342d1afa Mon Sep 17 00:00:00 2001
From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com>
Date: Thu, 30 Jan 2025 15:39:41 -0800
Subject: [PATCH 1/8] ref(tsc): convert orgDashboards to FC
---
static/app/utils/useParams.tsx | 1 +
static/app/views/dashboards/index.tsx | 10 +-
.../views/dashboards/orgDashboards.spec.tsx | 102 +++---
static/app/views/dashboards/orgDashboards.tsx | 292 +++++++++---------
static/app/views/dashboards/view.tsx | 7 +-
5 files changed, 193 insertions(+), 219 deletions(-)
diff --git a/static/app/utils/useParams.tsx b/static/app/utils/useParams.tsx
index f31392828def9f..fd4300979e5e16 100644
--- a/static/app/utils/useParams.tsx
+++ b/static/app/utils/useParams.tsx
@@ -17,6 +17,7 @@ type ParamKeys =
| 'authId'
| 'codeId'
| 'dataExportId'
+ | 'dashboardId'
| 'docIntegrationSlug'
| 'eventId'
| 'fineTuneType'
diff --git a/static/app/views/dashboards/index.tsx b/static/app/views/dashboards/index.tsx
index f34bd24a47fc97..d02e6ab2c4145c 100644
--- a/static/app/views/dashboards/index.tsx
+++ b/static/app/views/dashboards/index.tsx
@@ -21,21 +21,15 @@ type Props = RouteComponentProps<{}, {}> & {
};
function DashboardsV2Container(props: Props) {
- const {organization, api, location, children} = props;
+ const {organization, children} = props;
if (organization.features.includes('dashboards-edit')) {
return {children};
}
- const params = {...props.params, orgId: organization.slug};
return (
-
+
{({dashboard, dashboards, error, onDashboardUpdate}) => {
return error ? (
diff --git a/static/app/views/dashboards/orgDashboards.spec.tsx b/static/app/views/dashboards/orgDashboards.spec.tsx
index da5bd07180c448..90f5269cc813be 100644
--- a/static/app/views/dashboards/orgDashboards.spec.tsx
+++ b/static/app/views/dashboards/orgDashboards.spec.tsx
@@ -1,5 +1,6 @@
import {LocationFixture} from 'sentry-fixture/locationFixture';
import {OrganizationFixture} from 'sentry-fixture/organization';
+import {RouterFixture} from 'sentry-fixture/routerFixture';
import {initializeOrg} from 'sentry-test/initializeOrg';
import {render, screen, waitFor} from 'sentry-test/reactTestingLibrary';
@@ -52,6 +53,10 @@ describe('OrgDashboards', () => {
});
it('redirects to add query params for page filters if any are saved', async () => {
+ const router = RouterFixture({
+ location: LocationFixture(),
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
const mockDashboardWithFilters = {
dateCreated: '2021-08-10T21:20:46.798237Z',
id: '1',
@@ -72,12 +77,7 @@ describe('OrgDashboards', () => {
body: [mockDashboardWithFilters],
});
render(
-
+
{({dashboard, dashboards}) => {
return dashboard ? (
{
);
}}
,
- {router: initialData.router}
+ {router}
);
await waitFor(() =>
- expect(initialData.router.replace).toHaveBeenCalledWith(
+ expect(router.replace).toHaveBeenCalledWith(
expect.objectContaining({
query: expect.objectContaining({
project: [1, 2],
@@ -128,20 +128,19 @@ describe('OrgDashboards', () => {
url: '/organizations/org-slug/dashboards/',
body: [mockDashboardWithFilters],
});
+ const router = RouterFixture({
+ location: {
+ ...LocationFixture(),
+ query: {
+ // This query param is not a page filter, so it should not interfere
+ // with the redirect logic
+ sort: 'recentlyViewed',
+ },
+ },
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
render(
-
+
{({dashboard, dashboards}) => {
return dashboard ? (
{
);
}}
,
- {router: initialData.router}
+ {router}
);
await waitFor(() =>
- expect(initialData.router.replace).toHaveBeenCalledWith(
+ expect(router.replace).toHaveBeenCalledWith(
expect.objectContaining({
query: expect.objectContaining({
project: [1, 2],
@@ -206,13 +205,14 @@ describe('OrgDashboards', () => {
url: '/organizations/org-slug/dashboards/',
body: [mockDashboardWithFilters],
});
+
+ const router = RouterFixture({
+ location: LocationFixture(),
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
+
render(
-
+
{({dashboard, dashboards}) => {
return dashboard ? (
{
);
}}
,
- {router: initialData.router}
+ {router}
);
// The first call is done by the page filters
- expect(initialData.router.replace).not.toHaveBeenCalledTimes(2);
+ expect(router.replace).not.toHaveBeenCalledTimes(2);
});
it('does not add query params for page filters if none are saved', () => {
+ const router = RouterFixture({
+ location: LocationFixture(),
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
+
render(
-
+
{({dashboard, dashboards}) => {
return dashboard ? (
{
);
}}
,
- {router: initialData.router}
+ {router}
);
- expect(initialData.router.replace).not.toHaveBeenCalled();
+ expect(router.replace).not.toHaveBeenCalled();
});
it('does not redirect to add query params if location is cleared manually', async () => {
@@ -280,13 +280,14 @@ describe('OrgDashboards', () => {
url: '/organizations/org-slug/dashboards/',
body: [mockDashboardWithFilters],
});
+
+ const router = RouterFixture({
+ location: LocationFixture(),
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
+
const {rerender} = render(
-
+
{({dashboard, dashboards}) => {
return dashboard ? (
{
);
}}
,
- {router: initialData.router}
+ {router}
);
- await waitFor(() => expect(initialData.router.replace).toHaveBeenCalledTimes(1));
+ await waitFor(() => expect(router.replace).toHaveBeenCalledTimes(1));
rerender(
{({dashboard, dashboards}) => {
return dashboard ? (
@@ -330,6 +328,6 @@ describe('OrgDashboards', () => {
);
expect(screen.queryByTestId('loading-indicator')).not.toBeInTheDocument();
- expect(initialData.router.replace).toHaveBeenCalledTimes(1);
+ expect(router.replace).toHaveBeenCalledTimes(1);
});
});
diff --git a/static/app/views/dashboards/orgDashboards.tsx b/static/app/views/dashboards/orgDashboards.tsx
index 22dfb6382935e3..fb3717fcb4d1e7 100644
--- a/static/app/views/dashboards/orgDashboards.tsx
+++ b/static/app/views/dashboards/orgDashboards.tsx
@@ -1,18 +1,19 @@
-import type {Location} from 'history';
-import isEqual from 'lodash/isEqual';
+import {useState} from 'react';
-import type {Client} from 'sentry/api';
-import DeprecatedAsyncComponent from 'sentry/components/deprecatedAsyncComponent';
import NotFound from 'sentry/components/errors/notFound';
import * as Layout from 'sentry/components/layouts/thirds';
+import LoadingError from 'sentry/components/loadingError';
import LoadingIndicator from 'sentry/components/loadingIndicator';
import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
import {t} from 'sentry/locale';
-import type {Organization} from 'sentry/types/organization';
-import {browserHistory} from 'sentry/utils/browserHistory';
+import {useApiQuery} from 'sentry/utils/queryClient';
import type {WithRouteAnalyticsProps} from 'sentry/utils/routeAnalytics/withRouteAnalytics';
import withRouteAnalytics from 'sentry/utils/routeAnalytics/withRouteAnalytics';
import normalizeUrl from 'sentry/utils/url/normalizeUrl';
+import {useLocation} from 'sentry/utils/useLocation';
+import {useNavigate} from 'sentry/utils/useNavigate';
+import useOrganization from 'sentry/utils/useOrganization';
+import {useParams} from 'sentry/utils/useParams';
import {assignTempId} from './layoutUtils';
import type {DashboardDetails, DashboardListItem} from './types';
@@ -26,133 +27,130 @@ type OrgDashboardsChildrenProps = {
};
type Props = WithRouteAnalyticsProps & {
- api: Client;
children: (props: OrgDashboardsChildrenProps) => React.ReactNode;
- location: Location;
- organization: Organization;
- params: {orgId: string; dashboardId?: string};
};
-type State = {
- // endpoint response
- dashboards: DashboardListItem[] | null;
- /**
- * The currently selected dashboard.
- */
- selectedDashboard: DashboardDetails | null;
-} & DeprecatedAsyncComponent['state'];
-
-class OrgDashboards extends DeprecatedAsyncComponent {
- state: State = {
- // AsyncComponent state
- loading: true,
- reloading: false,
- error: false,
- errors: {},
-
- dashboards: [],
- selectedDashboard: null,
- };
-
- componentDidUpdate(prevProps: Props) {
- if (!isEqual(prevProps.params.dashboardId, this.props.params.dashboardId)) {
- this.remountComponent();
- }
+function OrgDashboards(props: Props) {
+ const {children} = props;
+ const location = useLocation();
+ const organization = useOrganization();
+ const navigate = useNavigate();
+ const {dashboardId} = useParams<{dashboardId: string}>();
+
+ const ENDPOINT = `/organizations/${organization.slug}/dashboards/`;
+
+ // The currently selected dashboard
+ const [selectedDashboardState, setSelectedDashboardState] =
+ useState(null);
+
+ // componentDidUpdate(prevProps: Props) {
+ // if (!isEqual(prevProps.params.dashboardId, this.props.params.dashboardId)) {
+ // this.remountComponent();
+ // }
+ // }
+
+ const {
+ data: dashboards,
+ isPending: isDashboardsPending,
+ isError: isDashboardsError,
+ error: dashboardsError,
+ } = useApiQuery([ENDPOINT], {staleTime: 0});
+
+ const {
+ data: fetchedSelectedDashboard,
+ isPending: isSelectedDashboardPending,
+ isError: isSelectedDashboardError,
+ error: selectedDashboardError,
+ } = useApiQuery([`${ENDPOINT}${dashboardId}/`], {
+ staleTime: 0,
+ enabled: !!dashboardId,
+ });
+
+ if (isDashboardsPending || isSelectedDashboardPending) {
+ return (
+
+
+
+ );
}
- getEndpoints(): ReturnType {
- const {organization, params} = this.props;
- const url = `/organizations/${organization.slug}/dashboards/`;
- const endpoints: ReturnType = [
- ['dashboards', url],
- ];
+ if (isDashboardsError || isSelectedDashboardError) {
+ const notFound =
+ dashboardsError?.status === 404 || selectedDashboardError?.status === 404;
- if (params.dashboardId) {
- endpoints.push(['selectedDashboard', `${url}${params.dashboardId}/`]);
+ if (notFound) {
+ return ;
}
- return endpoints;
+ return ;
}
- onDashboardUpdate(updatedDashboard: DashboardDetails) {
- this.setState({selectedDashboard: updatedDashboard});
- }
+ const selectedDashboard = selectedDashboardState ?? fetchedSelectedDashboard;
- getDashboards(): DashboardListItem[] {
- const {dashboards} = this.state;
+ const onDashboardUpdate = (updatedDashboard: DashboardDetails) => {
+ setSelectedDashboardState(updatedDashboard);
+ };
+ const getDashboards = (): DashboardListItem[] => {
return Array.isArray(dashboards) ? dashboards : [];
- }
+ };
- onRequestSuccess({stateKey, data}: any) {
- const {params, organization, location} = this.props;
-
- if (params.dashboardId || stateKey === 'selectedDashboard') {
- const queryParamFilters = new Set([
- 'project',
- 'environment',
- 'statsPeriod',
- 'start',
- 'end',
- 'utc',
- 'release',
- ]);
- if (
- stateKey === 'selectedDashboard' &&
- // Only redirect if there are saved filters and none of the filters
- // appear in the query params
- hasSavedPageFilters(data) &&
- Object.keys(location.query).filter(unsavedQueryParam =>
- queryParamFilters.has(unsavedQueryParam)
- ).length === 0
- ) {
- browserHistory.replace({
+ if (dashboardId || fetchedSelectedDashboard) {
+ const queryParamFilters = new Set([
+ 'project',
+ 'environment',
+ 'statsPeriod',
+ 'start',
+ 'end',
+ 'utc',
+ 'release',
+ ]);
+ if (
+ fetchedSelectedDashboard &&
+ // Only redirect if there are saved filters and none of the filters
+ // appear in the query params
+ hasSavedPageFilters(fetchedSelectedDashboard) &&
+ Object.keys(location.query).filter(unsavedQueryParam =>
+ queryParamFilters.has(unsavedQueryParam)
+ ).length === 0
+ ) {
+ navigate(
+ {
...location,
query: {
...location.query,
- project: data.projects,
- environment: data.environment,
- statsPeriod: data.period,
- start: data.start,
- end: data.end,
- utc: data.utc,
+ project: fetchedSelectedDashboard.projects,
+ environment: fetchedSelectedDashboard.environment,
+ statsPeriod: fetchedSelectedDashboard.period,
+ start: fetchedSelectedDashboard.start,
+ end: fetchedSelectedDashboard.end,
+ utc: fetchedSelectedDashboard.utc,
},
- });
- }
- return;
- }
-
- // If we don't have a selected dashboard, and one isn't going to arrive
- // we can redirect to the first dashboard in the list.
- const dashboardId = data.length ? data[0].id : 'default-overview';
- browserHistory.replace(
- normalizeUrl({
- pathname: `/organizations/${organization.slug}/dashboard/${dashboardId}/`,
- query: {
- ...location.query,
},
- })
- );
- }
-
- renderLoading() {
- return (
-
-
-
- );
+ {replace: true}
+ );
+ }
}
- renderBody() {
- const {children} = this.props;
- const {selectedDashboard, error} = this.state;
- let dashboard = selectedDashboard;
-
+ // If we don't have a selected dashboard, and one isn't going to arrive
+ // we can redirect to the first dashboard in the list.
+ const firstDashboardId = dashboards.length ? dashboards[0]?.id : 'default-overview';
+ navigate(
+ normalizeUrl({
+ pathname: `/organizations/${organization.slug}/dashboard/${firstDashboardId}/`,
+ query: {
+ ...location.query,
+ },
+ }),
+ {replace: true}
+ );
+
+ const renderContent = () => {
// Ensure there are always tempIds for grid layout
// This is needed because there are cases where the dashboard
// renders before the onRequestSuccess setState is processed
// and will caused stacked widgets because of missing tempIds
- dashboard = selectedDashboard
+ const dashboard = selectedDashboard
? {
...selectedDashboard,
widgets: selectedDashboard.widgets.map(assignTempId),
@@ -160,61 +158,49 @@ class OrgDashboards extends DeprecatedAsyncComponent {
: null;
return children({
- error,
+ error: Boolean(dashboardsError || selectedDashboardError),
dashboard,
- dashboards: this.getDashboards(),
+ dashboards: getDashboards(),
onDashboardUpdate: (updatedDashboard: DashboardDetails) =>
- this.onDashboardUpdate(updatedDashboard),
+ onDashboardUpdate(updatedDashboard),
});
- }
+ };
- renderError(error: Error) {
- const notFound = Object.values(this.state.errors).find(
- resp => resp && resp.status === 404
+ if (!organization.features.includes('dashboards-basic')) {
+ // Redirect to Dashboards v1
+ navigate(
+ normalizeUrl({
+ pathname: `/organizations/${organization.slug}/dashboards/`,
+ query: {
+ ...location.query,
+ },
+ }),
+ {replace: true}
);
-
- if (notFound) {
- return ;
- }
-
- return super.renderError(error, true);
+ return null;
}
- renderComponent() {
- const {organization, location} = this.props;
- const {loading, selectedDashboard} = this.state;
-
- if (!organization.features.includes('dashboards-basic')) {
- // Redirect to Dashboards v1
- browserHistory.replace(
- normalizeUrl({
- pathname: `/organizations/${organization.slug}/dashboards/`,
- query: {
- ...location.query,
- },
- })
- );
- return null;
- }
-
- if (
- loading &&
- selectedDashboard &&
- hasSavedPageFilters(selectedDashboard) &&
- Object.keys(location.query).length === 0
- ) {
- // Block dashboard from rendering if the dashboard has filters and
- // the URL does not contain filters yet. The filters can either match the
- // saved filters, or can be different (i.e. sharing an unsaved state)
- return this.renderLoading();
- }
-
+ if (
+ (isDashboardsPending || isSelectedDashboardPending) &&
+ selectedDashboard &&
+ hasSavedPageFilters(selectedDashboard) &&
+ Object.keys(location.query).length === 0
+ ) {
+ // Block dashboard from rendering if the dashboard has filters and
+ // the URL does not contain filters yet. The filters can either match the
+ // saved filters, or can be different (i.e. sharing an unsaved state)
return (
-
- {super.renderComponent() as React.ReactChild}
-
+
+
+
);
}
+
+ return (
+
+ {renderContent()}
+
+ );
}
export default withRouteAnalytics(OrgDashboards);
diff --git a/static/app/views/dashboards/view.tsx b/static/app/views/dashboards/view.tsx
index ea413665c245d9..9f9257d53a6004 100644
--- a/static/app/views/dashboards/view.tsx
+++ b/static/app/views/dashboards/view.tsx
@@ -69,12 +69,7 @@ function ViewEditDashboard(props: Props) {
return (
-
+
{({dashboard, dashboards, error, onDashboardUpdate}) => {
return error ? (
From 841c44c223d0cfca208785a5149fb2e434f50aee Mon Sep 17 00:00:00 2001
From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com>
Date: Thu, 30 Jan 2025 15:55:05 -0800
Subject: [PATCH 2/8] :recycle: getQueryData
---
static/app/views/dashboards/orgDashboards.tsx | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/static/app/views/dashboards/orgDashboards.tsx b/static/app/views/dashboards/orgDashboards.tsx
index fb3717fcb4d1e7..a6a87a04bc69b4 100644
--- a/static/app/views/dashboards/orgDashboards.tsx
+++ b/static/app/views/dashboards/orgDashboards.tsx
@@ -6,7 +6,7 @@ import LoadingError from 'sentry/components/loadingError';
import LoadingIndicator from 'sentry/components/loadingIndicator';
import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
import {t} from 'sentry/locale';
-import {useApiQuery} from 'sentry/utils/queryClient';
+import {useApiQuery, useQueryClient} from 'sentry/utils/queryClient';
import type {WithRouteAnalyticsProps} from 'sentry/utils/routeAnalytics/withRouteAnalytics';
import withRouteAnalytics from 'sentry/utils/routeAnalytics/withRouteAnalytics';
import normalizeUrl from 'sentry/utils/url/normalizeUrl';
@@ -36,6 +36,7 @@ function OrgDashboards(props: Props) {
const organization = useOrganization();
const navigate = useNavigate();
const {dashboardId} = useParams<{dashboardId: string}>();
+ const queryClient = useQueryClient();
const ENDPOINT = `/organizations/${organization.slug}/dashboards/`;
@@ -87,15 +88,13 @@ function OrgDashboards(props: Props) {
const selectedDashboard = selectedDashboardState ?? fetchedSelectedDashboard;
- const onDashboardUpdate = (updatedDashboard: DashboardDetails) => {
- setSelectedDashboardState(updatedDashboard);
- };
-
const getDashboards = (): DashboardListItem[] => {
return Array.isArray(dashboards) ? dashboards : [];
};
- if (dashboardId || fetchedSelectedDashboard) {
+ const selectedDashboardStateKey = queryClient.getQueryData([ENDPOINT]);
+
+ if (dashboardId || selectedDashboardStateKey) {
const queryParamFilters = new Set([
'project',
'environment',
@@ -106,7 +105,7 @@ function OrgDashboards(props: Props) {
'release',
]);
if (
- fetchedSelectedDashboard &&
+ selectedDashboardStateKey &&
// Only redirect if there are saved filters and none of the filters
// appear in the query params
hasSavedPageFilters(fetchedSelectedDashboard) &&
@@ -161,8 +160,7 @@ function OrgDashboards(props: Props) {
error: Boolean(dashboardsError || selectedDashboardError),
dashboard,
dashboards: getDashboards(),
- onDashboardUpdate: (updatedDashboard: DashboardDetails) =>
- onDashboardUpdate(updatedDashboard),
+ onDashboardUpdate: setSelectedDashboardState,
});
};
From b69723f14dd62d639a14c28c79be0f552be7a965 Mon Sep 17 00:00:00 2001
From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com>
Date: Thu, 30 Jan 2025 21:45:13 -0800
Subject: [PATCH 3/8] :recycle: better
---
static/app/views/dashboards/orgDashboards.tsx | 127 +++++++++---------
1 file changed, 64 insertions(+), 63 deletions(-)
diff --git a/static/app/views/dashboards/orgDashboards.tsx b/static/app/views/dashboards/orgDashboards.tsx
index a6a87a04bc69b4..88286d1f706405 100644
--- a/static/app/views/dashboards/orgDashboards.tsx
+++ b/static/app/views/dashboards/orgDashboards.tsx
@@ -1,4 +1,5 @@
-import {useState} from 'react';
+import {useEffect, useState} from 'react';
+import isEqual from 'lodash/isEqual';
import NotFound from 'sentry/components/errors/notFound';
import * as Layout from 'sentry/components/layouts/thirds';
@@ -6,7 +7,7 @@ import LoadingError from 'sentry/components/loadingError';
import LoadingIndicator from 'sentry/components/loadingIndicator';
import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
import {t} from 'sentry/locale';
-import {useApiQuery, useQueryClient} from 'sentry/utils/queryClient';
+import {useApiQuery} from 'sentry/utils/queryClient';
import type {WithRouteAnalyticsProps} from 'sentry/utils/routeAnalytics/withRouteAnalytics';
import withRouteAnalytics from 'sentry/utils/routeAnalytics/withRouteAnalytics';
import normalizeUrl from 'sentry/utils/url/normalizeUrl';
@@ -36,7 +37,6 @@ function OrgDashboards(props: Props) {
const organization = useOrganization();
const navigate = useNavigate();
const {dashboardId} = useParams<{dashboardId: string}>();
- const queryClient = useQueryClient();
const ENDPOINT = `/organizations/${organization.slug}/dashboards/`;
@@ -44,12 +44,6 @@ function OrgDashboards(props: Props) {
const [selectedDashboardState, setSelectedDashboardState] =
useState(null);
- // componentDidUpdate(prevProps: Props) {
- // if (!isEqual(prevProps.params.dashboardId, this.props.params.dashboardId)) {
- // this.remountComponent();
- // }
- // }
-
const {
data: dashboards,
isPending: isDashboardsPending,
@@ -67,6 +61,67 @@ function OrgDashboards(props: Props) {
enabled: !!dashboardId,
});
+ const selectedDashboard = selectedDashboardState ?? fetchedSelectedDashboard;
+
+ useEffect(() => {
+ if (dashboardId && !isEqual(dashboardId, selectedDashboardState?.id)) {
+ setSelectedDashboardState(null);
+ }
+ }, [dashboardId, selectedDashboardState]);
+
+ // If we don't have a selected dashboard, and one isn't going to arrive
+ // we can redirect to the first dashboard in the list.
+ useEffect(() => {
+ const firstDashboardId = dashboards?.length ? dashboards[0]?.id : 'default-overview';
+ navigate(
+ normalizeUrl({
+ pathname: `/organizations/${organization.slug}/dashboard/${firstDashboardId}/`,
+ query: {
+ ...location.query,
+ },
+ }),
+ {replace: true}
+ );
+ }, [dashboards, organization.slug, location.query, navigate]);
+
+ useEffect(() => {
+ if (selectedDashboard) {
+ const queryParamFilters = new Set([
+ 'project',
+ 'environment',
+ 'statsPeriod',
+ 'start',
+ 'end',
+ 'utc',
+ 'release',
+ ]);
+ if (
+ // Only redirect if there are saved filters and none of the filters
+ // appear in the query params
+ hasSavedPageFilters(selectedDashboard) &&
+ Object.keys(location.query).filter(unsavedQueryParam =>
+ queryParamFilters.has(unsavedQueryParam)
+ ).length === 0
+ ) {
+ navigate(
+ {
+ ...location,
+ query: {
+ ...location.query,
+ project: selectedDashboard.projects,
+ environment: selectedDashboard.environment,
+ statsPeriod: selectedDashboard.period,
+ start: selectedDashboard.start,
+ end: selectedDashboard.end,
+ utc: selectedDashboard.utc,
+ },
+ },
+ {replace: true}
+ );
+ }
+ }
+ }, [dashboardId, location, navigate, selectedDashboard]);
+
if (isDashboardsPending || isSelectedDashboardPending) {
return (
@@ -86,64 +141,10 @@ function OrgDashboards(props: Props) {
return ;
}
- const selectedDashboard = selectedDashboardState ?? fetchedSelectedDashboard;
-
const getDashboards = (): DashboardListItem[] => {
return Array.isArray(dashboards) ? dashboards : [];
};
- const selectedDashboardStateKey = queryClient.getQueryData([ENDPOINT]);
-
- if (dashboardId || selectedDashboardStateKey) {
- const queryParamFilters = new Set([
- 'project',
- 'environment',
- 'statsPeriod',
- 'start',
- 'end',
- 'utc',
- 'release',
- ]);
- if (
- selectedDashboardStateKey &&
- // Only redirect if there are saved filters and none of the filters
- // appear in the query params
- hasSavedPageFilters(fetchedSelectedDashboard) &&
- Object.keys(location.query).filter(unsavedQueryParam =>
- queryParamFilters.has(unsavedQueryParam)
- ).length === 0
- ) {
- navigate(
- {
- ...location,
- query: {
- ...location.query,
- project: fetchedSelectedDashboard.projects,
- environment: fetchedSelectedDashboard.environment,
- statsPeriod: fetchedSelectedDashboard.period,
- start: fetchedSelectedDashboard.start,
- end: fetchedSelectedDashboard.end,
- utc: fetchedSelectedDashboard.utc,
- },
- },
- {replace: true}
- );
- }
- }
-
- // If we don't have a selected dashboard, and one isn't going to arrive
- // we can redirect to the first dashboard in the list.
- const firstDashboardId = dashboards.length ? dashboards[0]?.id : 'default-overview';
- navigate(
- normalizeUrl({
- pathname: `/organizations/${organization.slug}/dashboard/${firstDashboardId}/`,
- query: {
- ...location.query,
- },
- }),
- {replace: true}
- );
-
const renderContent = () => {
// Ensure there are always tempIds for grid layout
// This is needed because there are cases where the dashboard
From bf4370e0e2af468f5bb345e4866c59d9a4ef3045 Mon Sep 17 00:00:00 2001
From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com>
Date: Thu, 30 Jan 2025 22:29:42 -0800
Subject: [PATCH 4/8] :white_check_mark: fix one test file
---
.../views/dashboards/orgDashboards.spec.tsx | 14 ++-
static/app/views/dashboards/orgDashboards.tsx | 91 ++++++++++---------
2 files changed, 54 insertions(+), 51 deletions(-)
diff --git a/static/app/views/dashboards/orgDashboards.spec.tsx b/static/app/views/dashboards/orgDashboards.spec.tsx
index 90f5269cc813be..5bf36c7468d6ee 100644
--- a/static/app/views/dashboards/orgDashboards.spec.tsx
+++ b/static/app/views/dashboards/orgDashboards.spec.tsx
@@ -92,7 +92,7 @@ describe('OrgDashboards', () => {
);
}}
,
- {router}
+ {router, organization}
);
await waitFor(() =>
@@ -155,7 +155,7 @@ describe('OrgDashboards', () => {
);
}}
,
- {router}
+ {router, organization}
);
await waitFor(() =>
@@ -227,7 +227,7 @@ describe('OrgDashboards', () => {
);
}}
,
- {router}
+ {router, organization}
);
// The first call is done by the page filters
@@ -256,7 +256,7 @@ describe('OrgDashboards', () => {
);
}}
,
- {router}
+ {router, organization}
);
expect(router.replace).not.toHaveBeenCalled();
@@ -302,15 +302,13 @@ describe('OrgDashboards', () => {
);
}}
,
- {router}
+ {router, organization}
);
await waitFor(() => expect(router.replace).toHaveBeenCalledTimes(1));
rerender(
-
+
{({dashboard, dashboards}) => {
return dashboard ? (
{
- if (dashboardId && !isEqual(dashboardId, selectedDashboardState?.id)) {
+ if (dashboardId && !isEqual(dashboardId, selectedDashboard?.id)) {
setSelectedDashboardState(null);
}
- }, [dashboardId, selectedDashboardState]);
+ }, [dashboardId, selectedDashboard]);
// If we don't have a selected dashboard, and one isn't going to arrive
// we can redirect to the first dashboard in the list.
useEffect(() => {
- const firstDashboardId = dashboards?.length ? dashboards[0]?.id : 'default-overview';
- navigate(
- normalizeUrl({
- pathname: `/organizations/${organization.slug}/dashboard/${firstDashboardId}/`,
- query: {
- ...location.query,
- },
- }),
- {replace: true}
- );
- }, [dashboards, organization.slug, location.query, navigate]);
+ if (!dashboardId) {
+ const firstDashboardId = dashboards?.length
+ ? dashboards[0]?.id
+ : 'default-overview';
+ navigate(
+ normalizeUrl({
+ pathname: `/organizations/${organization.slug}/dashboard/${firstDashboardId}/`,
+ query: {
+ ...location.query,
+ },
+ }),
+ {replace: true}
+ );
+ }
+ }, [dashboards, dashboardId, organization.slug, location.query, navigate]);
useEffect(() => {
if (selectedDashboard) {
@@ -122,6 +126,21 @@ function OrgDashboards(props: Props) {
}
}, [dashboardId, location, navigate, selectedDashboard]);
+ useEffect(() => {
+ if (!organization.features.includes('dashboards-basic')) {
+ // Redirect to Dashboards v1
+ navigate(
+ normalizeUrl({
+ pathname: `/organizations/${organization.slug}/dashboards/`,
+ query: {
+ ...location.query,
+ },
+ }),
+ {replace: true}
+ );
+ }
+ }, [location.query, navigate, organization]);
+
if (isDashboardsPending || isSelectedDashboardPending) {
return (
@@ -130,6 +149,22 @@ function OrgDashboards(props: Props) {
);
}
+ if (
+ (isDashboardsPending || isSelectedDashboardPending) &&
+ selectedDashboard &&
+ hasSavedPageFilters(selectedDashboard) &&
+ Object.keys(location.query).length === 0
+ ) {
+ // Block dashboard from rendering if the dashboard has filters and
+ // the URL does not contain filters yet. The filters can either match the
+ // saved filters, or can be different (i.e. sharing an unsaved state)
+ return (
+
+
+
+ );
+ }
+
if (isDashboardsError || isSelectedDashboardError) {
const notFound =
dashboardsError?.status === 404 || selectedDashboardError?.status === 404;
@@ -165,36 +200,6 @@ function OrgDashboards(props: Props) {
});
};
- if (!organization.features.includes('dashboards-basic')) {
- // Redirect to Dashboards v1
- navigate(
- normalizeUrl({
- pathname: `/organizations/${organization.slug}/dashboards/`,
- query: {
- ...location.query,
- },
- }),
- {replace: true}
- );
- return null;
- }
-
- if (
- (isDashboardsPending || isSelectedDashboardPending) &&
- selectedDashboard &&
- hasSavedPageFilters(selectedDashboard) &&
- Object.keys(location.query).length === 0
- ) {
- // Block dashboard from rendering if the dashboard has filters and
- // the URL does not contain filters yet. The filters can either match the
- // saved filters, or can be different (i.e. sharing an unsaved state)
- return (
-
-
-
- );
- }
-
return (
{renderContent()}
From 9c41157e5c17426f37b0aeb8406ff60e9c8c4adf Mon Sep 17 00:00:00 2001
From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com>
Date: Thu, 30 Jan 2025 23:22:49 -0800
Subject: [PATCH 5/8] :white_check_mark: fix tests
---
static/app/views/dashboards/detail.spec.tsx | 284 +++++++++++++++---
static/app/views/dashboards/orgDashboards.tsx | 3 +-
2 files changed, 245 insertions(+), 42 deletions(-)
diff --git a/static/app/views/dashboards/detail.spec.tsx b/static/app/views/dashboards/detail.spec.tsx
index 2dc90c2fb11564..698892065725cd 100644
--- a/static/app/views/dashboards/detail.spec.tsx
+++ b/static/app/views/dashboards/detail.spec.tsx
@@ -4,6 +4,7 @@ import {OrganizationFixture} from 'sentry-fixture/organization';
import {ProjectFixture} from 'sentry-fixture/project';
import {ReleaseFixture} from 'sentry-fixture/release';
import {RouteComponentPropsFixture} from 'sentry-fixture/routeComponentPropsFixture';
+import {RouterFixture} from 'sentry-fixture/routerFixture';
import {TeamFixture} from 'sentry-fixture/team';
import {UserFixture} from 'sentry-fixture/user';
import {WidgetFixture} from 'sentry-fixture/widget';
@@ -15,6 +16,7 @@ import {
screen,
userEvent,
waitFor,
+ waitForElementToBeRemoved,
within,
} from 'sentry-test/reactTestingLibrary';
@@ -123,6 +125,13 @@ describe('Dashboards > Detail', function () {
});
it('assigns unique IDs to all widgets so grid keys are unique', async function () {
+ const router = RouterFixture({
+ location: initialData.router.location,
+ params: {orgId: 'org-slug', dashboardId: 'default-overview'},
+ });
+ MockApiClient.addMockResponse({
+ url: '/organizations/org-slug/dashboards/1/',
+ });
MockApiClient.addMockResponse({
url: '/organizations/org-slug/events-stats/',
body: {data: []},
@@ -181,7 +190,7 @@ describe('Dashboards > Detail', function () {
{null}
,
- {router: initialData.router}
+ {router}
);
expect(await screen.findByText('Default Widget 1')).toBeInTheDocument();
@@ -411,6 +420,10 @@ describe('Dashboards > Detail', function () {
});
it('can remove widgets', async function () {
+ const router = RouterFixture({
+ location: initialData.router.location,
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
const updateMock = MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
method: 'PUT',
@@ -428,7 +441,7 @@ describe('Dashboards > Detail', function () {
{null}
,
- {router: initialData.router}
+ {router}
);
await waitFor(() => expect(mockVisit).toHaveBeenCalledTimes(1));
@@ -463,6 +476,10 @@ describe('Dashboards > Detail', function () {
});
it('appends dashboard-level filters to series request', async function () {
+ const router = RouterFixture({
+ location: initialData.router.location,
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
body: DashboardFixture(widgets, {
@@ -488,7 +505,7 @@ describe('Dashboards > Detail', function () {
{null}
,
- {router: initialData.router}
+ {router}
);
await waitFor(() =>
@@ -505,6 +522,10 @@ describe('Dashboards > Detail', function () {
});
it('shows add widget option', async function () {
+ const router = RouterFixture({
+ location: initialData.router.location,
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
render(
Detail', function () {
{null}
,
- {router: initialData.router}
+ {router}
);
// Enter edit mode.
- await userEvent.click(screen.getByRole('button', {name: 'Edit Dashboard'}));
+ await userEvent.click(await screen.findByRole('button', {name: 'Edit Dashboard'}));
expect(await screen.findByRole('button', {name: 'Add widget'})).toBeInTheDocument();
});
it('shows add widget option with dataset selector flag', async function () {
+ const router = RouterFixture({
+ location: initialData.router.location,
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
initialData = initializeOrg({
organization: OrganizationFixture({
features: [
@@ -550,10 +575,11 @@ describe('Dashboards > Detail', function () {
{null}
,
- {router: initialData.router}
+ {router}
);
- await userEvent.click(screen.getAllByText('Add Widget')[0]!);
+ await waitForElementToBeRemoved(() => screen.queryByTestId('loading-indicator'));
+ await userEvent.click(await screen.getAllByText('Add Widget')[0]!);
const menuOptions = await screen.findAllByTestId('menu-list-item-label');
expect(menuOptions.map(e => e.textContent)).toEqual([
'Errors',
@@ -565,6 +591,10 @@ describe('Dashboards > Detail', function () {
});
it('shows add widget option without dataset selector flag', async function () {
+ const router = RouterFixture({
+ location: initialData.router.location,
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
initialData = initializeOrg({
organization: OrganizationFixture({
features: [
@@ -588,10 +618,11 @@ describe('Dashboards > Detail', function () {
{null}
,
- {router: initialData.router}
+ {router}
);
- await userEvent.click(screen.getAllByText('Add Widget')[0]!);
+ await waitForElementToBeRemoved(() => screen.queryByTestId('loading-indicator'));
+ await userEvent.click(await screen.getAllByText('Add Widget')[0]!);
const menuOptions = await screen.findAllByTestId('menu-list-item-label');
expect(menuOptions.map(e => e.textContent)).toEqual([
'Errors and Transactions',
@@ -602,6 +633,10 @@ describe('Dashboards > Detail', function () {
});
it('shows top level release filter', async function () {
+ const router = RouterFixture({
+ location: initialData.router.location,
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
const mockReleases = MockApiClient.addMockResponse({
url: '/organizations/org-slug/releases/',
body: [ReleaseFixture()],
@@ -630,13 +665,17 @@ describe('Dashboards > Detail', function () {
{null}
,
- {router: initialData.router}
+ {router}
);
expect(await screen.findByText('All Releases')).toBeInTheDocument();
expect(mockReleases).toHaveBeenCalledTimes(2); // Called once when PageFiltersStore is initialized
});
it('hides add widget option', async function () {
+ const router = RouterFixture({
+ location: initialData.router.location,
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
// @ts-expect-error this is assigning to readonly property...
types.MAX_WIDGETS = 1;
@@ -652,7 +691,7 @@ describe('Dashboards > Detail', function () {
{null}
,
- {router: initialData.router}
+ {router}
);
// Enter edit mode.
@@ -661,6 +700,10 @@ describe('Dashboards > Detail', function () {
});
it('renders successfully if more widgets than stored layouts', async function () {
+ const router = RouterFixture({
+ location: initialData.router.location,
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
// A case where someone has async added widgets to a dashboard
MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
@@ -712,7 +755,7 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: initialData.router, organization: initialData.organization}
+ {router, organization: initialData.organization}
);
expect(await screen.findByText('First Widget')).toBeInTheDocument();
@@ -720,6 +763,10 @@ describe('Dashboards > Detail', function () {
});
it('does not trigger request if layout not updated', async () => {
+ const router = RouterFixture({
+ location: initialData.router.location,
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
body: DashboardFixture(
@@ -755,7 +802,7 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: initialData.router, organization: initialData.organization}
+ {router, organization: initialData.organization}
);
await userEvent.click(await screen.findByText('Edit Dashboard'));
@@ -766,6 +813,10 @@ describe('Dashboards > Detail', function () {
});
it('renders the custom resize handler for a widget', async () => {
+ const router = RouterFixture({
+ location: initialData.router.location,
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
body: DashboardFixture(
@@ -801,7 +852,7 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: initialData.router, organization: initialData.organization}
+ {router, organization: initialData.organization}
);
await userEvent.click(await screen.findByText('Edit Dashboard'));
@@ -814,6 +865,11 @@ describe('Dashboards > Detail', function () {
});
it('does not trigger an alert when the widgets have no layout and user cancels without changes', async () => {
+ const router = RouterFixture({
+ location: LocationFixture(),
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
+
MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
body: DashboardFixture(
@@ -849,7 +905,7 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: initialData.router, organization: initialData.organization}
+ {router, organization: initialData.organization}
);
await userEvent.click(await screen.findByText('Edit Dashboard'));
@@ -859,6 +915,10 @@ describe('Dashboards > Detail', function () {
});
it('opens the widget viewer modal using the widget id specified in the url', async () => {
+ const router = RouterFixture({
+ location: {...initialData.router.location, pathname: '/widget/123/'},
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
const openWidgetViewerModal = jest.spyOn(modals, 'openWidgetViewerModal');
const widget = WidgetFixture({
queries: [
@@ -891,7 +951,7 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: initialData.router, organization: initialData.organization}
+ {router, organization: initialData.organization}
);
await waitFor(() => {
@@ -906,6 +966,13 @@ describe('Dashboards > Detail', function () {
});
it('redirects user to dashboard url if widget is not found', async () => {
+ const router = RouterFixture({
+ location: {
+ ...initialData.router.location,
+ pathname: '/widget/123/',
+ },
+ params: {orgId: 'org-slug', dashboardId: '1', widgetId: 123},
+ });
const openWidgetViewerModal = jest.spyOn(modals, 'openWidgetViewerModal');
MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
@@ -921,7 +988,7 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: initialData.router, organization: initialData.organization}
+ {router, organization: initialData.organization}
);
expect(await screen.findByText('All Releases')).toBeInTheDocument();
@@ -1056,6 +1123,11 @@ describe('Dashboards > Detail', function () {
});
it('opens the widget viewer with saved dashboard filters', async () => {
+ const router = RouterFixture({
+ location: {...initialData.router.location, pathname: '/widget/1/'},
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
+
const openWidgetViewerModal = jest.spyOn(modals, 'openWidgetViewerModal');
MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
@@ -1074,7 +1146,7 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: initialData.router, organization: initialData.organization}
+ {router, organization: initialData.organization}
);
await waitFor(() => {
@@ -1087,6 +1159,15 @@ describe('Dashboards > Detail', function () {
});
it('opens the widget viewer with unsaved dashboard filters', async () => {
+ const router = RouterFixture({
+ location: {
+ ...initialData.router.location,
+ pathname: '/widget/1/',
+ query: {release: ['unsaved-release-filter@1.2.0']},
+ },
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
+
const openWidgetViewerModal = jest.spyOn(modals, 'openWidgetViewerModal');
MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
@@ -1109,7 +1190,7 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: initialData.router, organization: initialData.organization}
+ {router, organization: initialData.organization}
);
await waitFor(() => {
@@ -1122,6 +1203,17 @@ describe('Dashboards > Detail', function () {
});
it('can save dashboard filters in existing dashboard', async () => {
+ const router = RouterFixture({
+ location: {
+ ...LocationFixture(),
+ query: {
+ statsPeriod: '7d',
+ release: ['sentry-android-shop@1.2.0'],
+ },
+ },
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
+
MockApiClient.addMockResponse({
url: '/organizations/org-slug/releases/',
body: [
@@ -1160,7 +1252,7 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: testData.router, organization: testData.organization}
+ {router, organization: testData.organization}
);
await userEvent.click(await screen.findByText('Save'));
@@ -1177,6 +1269,16 @@ describe('Dashboards > Detail', function () {
});
it('can clear dashboard filters in compact select', async () => {
+ const router = RouterFixture({
+ location: {
+ ...LocationFixture(),
+ query: {
+ statsPeriod: '7d',
+ },
+ },
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
+
MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
body: DashboardFixture(widgets, {
@@ -1223,7 +1325,7 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: testData.router, organization: testData.organization}
+ {router, organization: testData.organization}
);
await screen.findByText('7D');
@@ -1233,7 +1335,7 @@ describe('Dashboards > Detail', function () {
await userEvent.click(document.body);
await waitFor(() => {
- expect(testData.router.push).toHaveBeenCalledWith(
+ expect(router.push).toHaveBeenCalledWith(
expect.objectContaining({
query: expect.objectContaining({
release: '',
@@ -1244,6 +1346,17 @@ describe('Dashboards > Detail', function () {
});
it('can save absolute time range in existing dashboard', async () => {
+ const router = RouterFixture({
+ location: {
+ ...LocationFixture(),
+ query: {
+ start: '2022-07-14T07:00:00',
+ end: '2022-07-19T23:59:59',
+ utc: 'true',
+ },
+ },
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
const testData = initializeOrg({
organization: OrganizationFixture({
features: [
@@ -1274,7 +1387,7 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: testData.router, organization: testData.organization}
+ {router, organization: testData.organization}
);
await userEvent.click(await screen.findByText('Save'));
@@ -1292,6 +1405,16 @@ describe('Dashboards > Detail', function () {
});
it('can clear dashboard filters in existing dashboard', async () => {
+ const router = RouterFixture({
+ location: {
+ ...LocationFixture(),
+ query: {
+ statsPeriod: '7d',
+ environment: ['alpha', 'beta'],
+ },
+ },
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
MockApiClient.addMockResponse({
url: '/organizations/org-slug/releases/',
body: [
@@ -1330,7 +1453,7 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: testData.router, organization: testData.organization}
+ {router, organization: testData.organization}
);
await screen.findByText('7D');
@@ -1341,7 +1464,7 @@ describe('Dashboards > Detail', function () {
await userEvent.click(screen.getByTestId('filter-bar-cancel'));
screen.getByText('All Releases');
- expect(testData.router.replace).toHaveBeenCalledWith(
+ expect(router.replace).toHaveBeenCalledWith(
expect.objectContaining({
query: expect.objectContaining({
project: undefined,
@@ -1353,6 +1476,16 @@ describe('Dashboards > Detail', function () {
});
it('disables the Edit Dashboard button when there are unsaved filters', async () => {
+ const router = RouterFixture({
+ location: {
+ ...LocationFixture(),
+ query: {
+ statsPeriod: '7d',
+ environment: ['alpha', 'beta'],
+ },
+ },
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
MockApiClient.addMockResponse({
url: '/organizations/org-slug/releases/',
body: [
@@ -1392,7 +1525,7 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: testData.router, organization: testData.organization}
+ {router, organization: testData.organization}
);
expect(await screen.findByText('Save')).toBeInTheDocument();
@@ -1401,6 +1534,15 @@ describe('Dashboards > Detail', function () {
});
it('ignores the order of selection of page filters to render unsaved filters', async () => {
+ const router = RouterFixture({
+ location: {
+ ...LocationFixture(),
+ query: {
+ environment: ['beta', 'alpha'],
+ },
+ },
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
const testProjects = [
ProjectFixture({id: '1', name: 'first', environments: ['alpha', 'beta']}),
ProjectFixture({id: '2', name: 'second', environments: ['alpha', 'beta']}),
@@ -1449,7 +1591,7 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: testData.router, organization: testData.organization}
+ {router, organization: testData.organization}
);
await waitFor(() => expect(screen.queryAllByText('Loading\u2026')).toEqual([]));
@@ -1469,6 +1611,15 @@ describe('Dashboards > Detail', function () {
});
it('uses releases from the URL query params', async function () {
+ const router = RouterFixture({
+ location: {
+ ...LocationFixture(),
+ query: {
+ release: ['not-selected-1'],
+ },
+ },
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
const testData = initializeOrg({
organization: OrganizationFixture({
features: [
@@ -1497,7 +1648,7 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: testData.router, organization: testData.organization}
+ {router, organization: testData.organization}
);
await screen.findByText(/not-selected-1/);
@@ -1506,6 +1657,15 @@ describe('Dashboards > Detail', function () {
});
it('resets release in URL params', async function () {
+ const router = RouterFixture({
+ location: {
+ ...LocationFixture(),
+ query: {
+ release: ['not-selected-1'],
+ },
+ },
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
body: DashboardFixture(widgets, {
@@ -1544,14 +1704,14 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: testData.router, organization: testData.organization}
+ {router, organization: testData.organization}
);
await screen.findByText(/not-selected-1/);
await userEvent.click(screen.getByTestId('filter-bar-cancel'));
// release isn't used in the redirect
- expect(testData.router.replace).toHaveBeenCalledWith(
+ expect(router.replace).toHaveBeenCalledWith(
expect.objectContaining({
query: {
end: undefined,
@@ -1566,6 +1726,10 @@ describe('Dashboards > Detail', function () {
});
it('reflects selections in the release filter in the query params', async function () {
+ const router = RouterFixture({
+ location: LocationFixture(),
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
MockApiClient.addMockResponse({
url: '/organizations/org-slug/releases/',
body: [
@@ -1598,7 +1762,7 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: testData.router, organization: testData.organization}
+ {router, organization: testData.organization}
);
await userEvent.click(await screen.findByText('All Releases'));
@@ -1606,7 +1770,7 @@ describe('Dashboards > Detail', function () {
await userEvent.click(document.body);
await waitFor(() => {
- expect(testData.router.push).toHaveBeenCalledWith(
+ expect(router.push).toHaveBeenCalledWith(
expect.objectContaining({
query: expect.objectContaining({
release: ['sentry-android-shop@1.2.0'],
@@ -1617,6 +1781,10 @@ describe('Dashboards > Detail', function () {
});
it('persists release selections made during search requests that do not appear in default query', async function () {
+ const router = RouterFixture({
+ location: LocationFixture(),
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
// Default response
MockApiClient.addMockResponse({
url: '/organizations/org-slug/releases/',
@@ -1663,7 +1831,7 @@ describe('Dashboards > Detail', function () {
>
{null}
,
- {router: testData.router, organization: testData.organization}
+ {router, organization: testData.organization}
);
await userEvent.click(await screen.findByText('All Releases'));
@@ -1693,6 +1861,10 @@ describe('Dashboards > Detail', function () {
});
it('creates and updates new permissions for dashboard with no edit perms initialized', async function () {
+ const router = RouterFixture({
+ location: LocationFixture(),
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
const mockPUT = MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
method: 'PUT',
@@ -1710,7 +1882,7 @@ describe('Dashboards > Detail', function () {
{null}
,
{
- router: initialData.router,
+ router,
organization: initialData.organization,
}
);
@@ -1744,6 +1916,10 @@ describe('Dashboards > Detail', function () {
});
it('creator can update permissions for dashboard', async function () {
+ const router = RouterFixture({
+ location: LocationFixture(),
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
const mockPUT = MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
method: 'PUT',
@@ -1773,7 +1949,7 @@ describe('Dashboards > Detail', function () {
{null}
,
{
- router: initialData.router,
+ router,
organization: initialData.organization,
}
);
@@ -1807,6 +1983,10 @@ describe('Dashboards > Detail', function () {
});
it('creator can update permissions with teams for dashboard', async function () {
+ const router = RouterFixture({
+ location: LocationFixture(),
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
const mockPUT = MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
method: 'PUT',
@@ -1857,7 +2037,7 @@ describe('Dashboards > Detail', function () {
{null}
,
{
- router: initialData.router,
+ router,
organization: initialData.organization,
}
);
@@ -1886,6 +2066,11 @@ describe('Dashboards > Detail', function () {
});
it('disables edit dashboard and add widget button if user cannot edit dashboard', async function () {
+ const router = RouterFixture({
+ location: LocationFixture(),
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
+
MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/',
body: [
@@ -1925,7 +2110,7 @@ describe('Dashboards > Detail', function () {
{null}
,
{
- router: initialData.router,
+ router,
organization: {
features: initialData.organization.features,
access: ['org:read'],
@@ -1939,6 +2124,10 @@ describe('Dashboards > Detail', function () {
});
it('disables widget edit, duplicate, and delete button when user does not have edit perms', async function () {
+ const router = RouterFixture({
+ location: LocationFixture(),
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
const widget = {
displayType: types.DisplayType.TABLE,
interval: '1d',
@@ -1989,7 +2178,7 @@ describe('Dashboards > Detail', function () {
{null}
,
{
- router: initialData.router,
+ router,
organization: {
features: initialData.organization.features,
access: ['org:read'],
@@ -2015,6 +2204,10 @@ describe('Dashboards > Detail', function () {
});
it('renders favorite button in unfavorited state', async function () {
+ const router = RouterFixture({
+ location: LocationFixture(),
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
body: DashboardFixture([], {id: '1', title: 'Custom Errors', isFavorited: false}),
@@ -2030,7 +2223,7 @@ describe('Dashboards > Detail', function () {
{null}
,
{
- router: initialData.router,
+ router,
organization: {
features: ['dashboards-favourite', ...initialData.organization.features],
},
@@ -2043,6 +2236,10 @@ describe('Dashboards > Detail', function () {
});
it('renders favorite button in favorited state', async function () {
+ const router = RouterFixture({
+ location: LocationFixture(),
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
body: DashboardFixture([], {id: '1', title: 'Custom Errors', isFavorited: true}),
@@ -2058,7 +2255,7 @@ describe('Dashboards > Detail', function () {
{null}
,
{
- router: initialData.router,
+ router,
organization: {
features: ['dashboards-favourite', ...initialData.organization.features],
},
@@ -2071,6 +2268,11 @@ describe('Dashboards > Detail', function () {
});
it('toggles favorite button', async function () {
+ const router = RouterFixture({
+ location: LocationFixture(),
+ params: {orgId: 'org-slug', dashboardId: '1'},
+ });
+
MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
body: DashboardFixture([], {id: '1', title: 'Custom Errors', isFavorited: true}),
@@ -2091,7 +2293,7 @@ describe('Dashboards > Detail', function () {
{null}
,
{
- router: initialData.router,
+ router,
organization: {
features: ['dashboards-favourite', ...initialData.organization.features],
},
diff --git a/static/app/views/dashboards/orgDashboards.tsx b/static/app/views/dashboards/orgDashboards.tsx
index 61bf4ecf371764..209c226174873a 100644
--- a/static/app/views/dashboards/orgDashboards.tsx
+++ b/static/app/views/dashboards/orgDashboards.tsx
@@ -89,7 +89,7 @@ function OrgDashboards(props: Props) {
}, [dashboards, dashboardId, organization.slug, location.query, navigate]);
useEffect(() => {
- if (selectedDashboard) {
+ if (dashboardId || selectedDashboard) {
const queryParamFilters = new Set([
'project',
'environment',
@@ -100,6 +100,7 @@ function OrgDashboards(props: Props) {
'release',
]);
if (
+ selectedDashboard &&
// Only redirect if there are saved filters and none of the filters
// appear in the query params
hasSavedPageFilters(selectedDashboard) &&
From 71ed953f00ce9374ff28bd87e14d765660dbec4d Mon Sep 17 00:00:00 2001
From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com>
Date: Fri, 31 Jan 2025 10:25:29 -0800
Subject: [PATCH 6/8] :recycle: rm extra
---
static/app/views/dashboards/detail.spec.tsx | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/static/app/views/dashboards/detail.spec.tsx b/static/app/views/dashboards/detail.spec.tsx
index 698892065725cd..4f1842eb98e731 100644
--- a/static/app/views/dashboards/detail.spec.tsx
+++ b/static/app/views/dashboards/detail.spec.tsx
@@ -129,9 +129,7 @@ describe('Dashboards > Detail', function () {
location: initialData.router.location,
params: {orgId: 'org-slug', dashboardId: 'default-overview'},
});
- MockApiClient.addMockResponse({
- url: '/organizations/org-slug/dashboards/1/',
- });
+
MockApiClient.addMockResponse({
url: '/organizations/org-slug/events-stats/',
body: {data: []},
From 3d2108484f72b87b68929259fa21485ddeffb4d6 Mon Sep 17 00:00:00 2001
From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com>
Date: Fri, 31 Jan 2025 10:33:59 -0800
Subject: [PATCH 7/8] :recycle: test fix
---
static/app/views/dashboards/detail.spec.tsx | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/static/app/views/dashboards/detail.spec.tsx b/static/app/views/dashboards/detail.spec.tsx
index 4f1842eb98e731..1c03853f160e01 100644
--- a/static/app/views/dashboards/detail.spec.tsx
+++ b/static/app/views/dashboards/detail.spec.tsx
@@ -981,7 +981,7 @@ describe('Dashboards > Detail', function () {
{...RouteComponentPropsFixture()}
organization={initialData.organization}
params={{orgId: 'org-slug', dashboardId: '1', widgetId: 123}}
- router={initialData.router}
+ router={router}
location={{...initialData.router.location, pathname: '/widget/123/'}}
>
{null}
@@ -992,7 +992,7 @@ describe('Dashboards > Detail', function () {
expect(await screen.findByText('All Releases')).toBeInTheDocument();
expect(openWidgetViewerModal).not.toHaveBeenCalled();
- expect(initialData.router.replace).toHaveBeenCalledWith(
+ expect(router.replace).toHaveBeenCalledWith(
expect.objectContaining({
pathname: '/organizations/org-slug/dashboard/1/',
query: {},
From 9dbbb182c3d733c2a2322d0781e76e7f4808902a Mon Sep 17 00:00:00 2001
From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com>
Date: Mon, 3 Feb 2025 16:28:54 -0800
Subject: [PATCH 8/8] :art: pr comments
---
static/app/views/dashboards/orgDashboards.tsx | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/static/app/views/dashboards/orgDashboards.tsx b/static/app/views/dashboards/orgDashboards.tsx
index 209c226174873a..9a66e6103106bd 100644
--- a/static/app/views/dashboards/orgDashboards.tsx
+++ b/static/app/views/dashboards/orgDashboards.tsx
@@ -49,16 +49,17 @@ function OrgDashboards(props: Props) {
isPending: isDashboardsPending,
isError: isDashboardsError,
error: dashboardsError,
- } = useApiQuery([ENDPOINT], {staleTime: 0});
+ } = useApiQuery([ENDPOINT], {staleTime: 0, retry: false});
const {
data: fetchedSelectedDashboard,
- isPending: isSelectedDashboardPending,
+ isLoading: isSelectedDashboardLoading,
isError: isSelectedDashboardError,
error: selectedDashboardError,
} = useApiQuery([`${ENDPOINT}${dashboardId}/`], {
staleTime: 0,
enabled: !!dashboardId,
+ retry: false,
});
const selectedDashboard = selectedDashboardState ?? fetchedSelectedDashboard;
@@ -142,7 +143,7 @@ function OrgDashboards(props: Props) {
}
}, [location.query, navigate, organization]);
- if (isDashboardsPending || isSelectedDashboardPending) {
+ if (isDashboardsPending || isSelectedDashboardLoading) {
return (
@@ -151,7 +152,7 @@ function OrgDashboards(props: Props) {
}
if (
- (isDashboardsPending || isSelectedDashboardPending) &&
+ (isDashboardsPending || isSelectedDashboardLoading) &&
selectedDashboard &&
hasSavedPageFilters(selectedDashboard) &&
Object.keys(location.query).length === 0