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/detail.spec.tsx b/static/app/views/dashboards/detail.spec.tsx index 2dc90c2fb11564..1c03853f160e01 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,11 @@ 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/events-stats/', body: {data: []}, @@ -181,7 +188,7 @@ describe('Dashboards > Detail', function () { {null} , - {router: initialData.router} + {router} ); expect(await screen.findByText('Default Widget 1')).toBeInTheDocument(); @@ -411,6 +418,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 +439,7 @@ describe('Dashboards > Detail', function () { {null} , - {router: initialData.router} + {router} ); await waitFor(() => expect(mockVisit).toHaveBeenCalledTimes(1)); @@ -463,6 +474,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 +503,7 @@ describe('Dashboards > Detail', function () { {null} , - {router: initialData.router} + {router} ); await waitFor(() => @@ -505,6 +520,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 +573,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 +589,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 +616,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 +631,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 +663,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 +689,7 @@ describe('Dashboards > Detail', function () { {null} , - {router: initialData.router} + {router} ); // Enter edit mode. @@ -661,6 +698,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 +753,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 +761,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 +800,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 +811,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 +850,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 +863,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 +903,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 +913,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 +949,7 @@ describe('Dashboards > Detail', function () { > {null} , - {router: initialData.router, organization: initialData.organization} + {router, organization: initialData.organization} ); await waitFor(() => { @@ -906,6 +964,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/', @@ -916,18 +981,18 @@ 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} , - {router: initialData.router, organization: initialData.organization} + {router, organization: initialData.organization} ); 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: {}, @@ -1056,6 +1121,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 +1144,7 @@ describe('Dashboards > Detail', function () { > {null} , - {router: initialData.router, organization: initialData.organization} + {router, organization: initialData.organization} ); await waitFor(() => { @@ -1087,6 +1157,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 +1188,7 @@ describe('Dashboards > Detail', function () { > {null} , - {router: initialData.router, organization: initialData.organization} + {router, organization: initialData.organization} ); await waitFor(() => { @@ -1122,6 +1201,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 +1250,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 +1267,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 +1323,7 @@ describe('Dashboards > Detail', function () { > {null} , - {router: testData.router, organization: testData.organization} + {router, organization: testData.organization} ); await screen.findByText('7D'); @@ -1233,7 +1333,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 +1344,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 +1385,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 +1403,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 +1451,7 @@ describe('Dashboards > Detail', function () { > {null} , - {router: testData.router, organization: testData.organization} + {router, organization: testData.organization} ); await screen.findByText('7D'); @@ -1341,7 +1462,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 +1474,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 +1523,7 @@ describe('Dashboards > Detail', function () { > {null} , - {router: testData.router, organization: testData.organization} + {router, organization: testData.organization} ); expect(await screen.findByText('Save')).toBeInTheDocument(); @@ -1401,6 +1532,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 +1589,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 +1609,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 +1646,7 @@ describe('Dashboards > Detail', function () { > {null} , - {router: testData.router, organization: testData.organization} + {router, organization: testData.organization} ); await screen.findByText(/not-selected-1/); @@ -1506,6 +1655,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 +1702,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 +1724,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 +1760,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 +1768,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 +1779,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 +1829,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 +1859,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 +1880,7 @@ describe('Dashboards > Detail', function () { {null} , { - router: initialData.router, + router, organization: initialData.organization, } ); @@ -1744,6 +1914,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 +1947,7 @@ describe('Dashboards > Detail', function () { {null} , { - router: initialData.router, + router, organization: initialData.organization, } ); @@ -1807,6 +1981,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 +2035,7 @@ describe('Dashboards > Detail', function () { {null} , { - router: initialData.router, + router, organization: initialData.organization, } ); @@ -1886,6 +2064,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 +2108,7 @@ describe('Dashboards > Detail', function () { {null} , { - router: initialData.router, + router, organization: { features: initialData.organization.features, access: ['org:read'], @@ -1939,6 +2122,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 +2176,7 @@ describe('Dashboards > Detail', function () { {null} , { - router: initialData.router, + router, organization: { features: initialData.organization.features, access: ['org:read'], @@ -2015,6 +2202,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 +2221,7 @@ describe('Dashboards > Detail', function () { {null} , { - router: initialData.router, + router, organization: { features: ['dashboards-favourite', ...initialData.organization.features], }, @@ -2043,6 +2234,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 +2253,7 @@ describe('Dashboards > Detail', function () { {null} , { - router: initialData.router, + router, organization: { features: ['dashboards-favourite', ...initialData.organization.features], }, @@ -2071,6 +2266,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 +2291,7 @@ describe('Dashboards > Detail', function () { {null} , { - router: initialData.router, + router, organization: { features: ['dashboards-favourite', ...initialData.organization.features], }, 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..5bf36c7468d6ee 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, organization} ); 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, organization} ); 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, organization} ); // 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, organization} ); - 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, organization} ); - await waitFor(() => expect(initialData.router.replace).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(router.replace).toHaveBeenCalledTimes(1)); rerender( - + {({dashboard, dashboards}) => { return dashboard ? ( { ); 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..9a66e6103106bd 100644 --- a/static/app/views/dashboards/orgDashboards.tsx +++ b/static/app/views/dashboards/orgDashboards.tsx @@ -1,18 +1,20 @@ -import type {Location} from 'history'; +import {useEffect, useState} from 'react'; import isEqual from 'lodash/isEqual'; -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,68 +28,69 @@ 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); + + const { + data: dashboards, + isPending: isDashboardsPending, + isError: isDashboardsError, + error: dashboardsError, + } = useApiQuery([ENDPOINT], {staleTime: 0, retry: false}); + + const { + data: fetchedSelectedDashboard, + isLoading: isSelectedDashboardLoading, + isError: isSelectedDashboardError, + error: selectedDashboardError, + } = useApiQuery([`${ENDPOINT}${dashboardId}/`], { + staleTime: 0, + enabled: !!dashboardId, + retry: false, + }); + + const selectedDashboard = selectedDashboardState ?? fetchedSelectedDashboard; + + useEffect(() => { + if (dashboardId && !isEqual(dashboardId, selectedDashboard?.id)) { + setSelectedDashboardState(null); } - } - - getEndpoints(): ReturnType { - const {organization, params} = this.props; - const url = `/organizations/${organization.slug}/dashboards/`; - const endpoints: ReturnType = [ - ['dashboards', url], - ]; - - if (params.dashboardId) { - endpoints.push(['selectedDashboard', `${url}${params.dashboardId}/`]); + }, [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(() => { + 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]); - return endpoints; - } - - onDashboardUpdate(updatedDashboard: DashboardDetails) { - this.setState({selectedDashboard: updatedDashboard}); - } - - getDashboards(): DashboardListItem[] { - const {dashboards} = this.state; - - return Array.isArray(dashboards) ? dashboards : []; - } - - onRequestSuccess({stateKey, data}: any) { - const {params, organization, location} = this.props; - - if (params.dashboardId || stateKey === 'selectedDashboard') { + useEffect(() => { + if (dashboardId || selectedDashboard) { const queryParamFilters = new Set([ 'project', 'environment', @@ -98,44 +101,65 @@ class OrgDashboards extends DeprecatedAsyncComponent { 'release', ]); if ( - stateKey === 'selectedDashboard' && + selectedDashboard && // Only redirect if there are saved filters and none of the filters // appear in the query params - hasSavedPageFilters(data) && + hasSavedPageFilters(selectedDashboard) && Object.keys(location.query).filter(unsavedQueryParam => queryParamFilters.has(unsavedQueryParam) ).length === 0 ) { - browserHistory.replace({ - ...location, + 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]); + + useEffect(() => { + if (!organization.features.includes('dashboards-basic')) { + // Redirect to Dashboards v1 + navigate( + normalizeUrl({ + pathname: `/organizations/${organization.slug}/dashboards/`, query: { ...location.query, - project: data.projects, - environment: data.environment, - statsPeriod: data.period, - start: data.start, - end: data.end, - utc: data.utc, }, - }); - } - return; + }), + {replace: true} + ); } + }, [location.query, navigate, organization]); - // 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, - }, - }) + if (isDashboardsPending || isSelectedDashboardLoading) { + return ( + + + ); } - renderLoading() { + if ( + (isDashboardsPending || isSelectedDashboardLoading) && + 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 ( @@ -143,16 +167,27 @@ class OrgDashboards extends DeprecatedAsyncComponent { ); } - renderBody() { - const {children} = this.props; - const {selectedDashboard, error} = this.state; - let dashboard = selectedDashboard; + if (isDashboardsError || isSelectedDashboardError) { + const notFound = + dashboardsError?.status === 404 || selectedDashboardError?.status === 404; + + if (notFound) { + return ; + } + + return ; + } + + const getDashboards = (): DashboardListItem[] => { + return Array.isArray(dashboards) ? dashboards : []; + }; + 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 +195,18 @@ class OrgDashboards extends DeprecatedAsyncComponent { : null; return children({ - error, + error: Boolean(dashboardsError || selectedDashboardError), dashboard, - dashboards: this.getDashboards(), - onDashboardUpdate: (updatedDashboard: DashboardDetails) => - this.onDashboardUpdate(updatedDashboard), + dashboards: getDashboards(), + onDashboardUpdate: setSelectedDashboardState, }); - } - - renderError(error: Error) { - const notFound = Object.values(this.state.errors).find( - resp => resp && resp.status === 404 - ); - - if (notFound) { - return ; - } - - return super.renderError(error, true); - } - - 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(); - } + }; - 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 ? (