fix(dashboards): Indicate 404/400/500 properly (#21853)

* Add stories for dashboard 404 and 500

* Rework `receivedErrorsFromAPI` into `dashboardFailedToLoad`

* Update FeatureFlag.tsx

* Update UI snapshots for `chromium` (1)

* Update dashboardLogic test

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (1)

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

frontend/src/scenes/dashboard/Dashboard.tsx

@@ -53,7 +53,7 @@ function DashboardScene(): JSX.Element {
         itemsLoading,
         filters: dashboardFilters,
         dashboardMode,
-        receivedErrorsFromAPI,
+        dashboardFailedToLoad,
     } = useValues(dashboardLogic)
     const { setDashboardMode, setDates, reportDashboardViewed, setProperties, abortAnyRunningQuery } =
         useActions(dashboardLogic)
@@ -96,15 +96,15 @@ function DashboardScene(): JSX.Element {
         [setDashboardMode, dashboardMode, placement]
     )
 
-    if (!dashboard && !itemsLoading && !receivedErrorsFromAPI) {
+    if (!dashboard && !itemsLoading && !dashboardFailedToLoad) {
         return <NotFound object="dashboard" />
     }
 
     return (
         <div className="dashboard">
             {placement == DashboardPlacement.Dashboard && <DashboardHeader />}
 
-            {receivedErrorsFromAPI ? (
+            {dashboardFailedToLoad ? (
                 <InsightErrorState title="There was an error loading this dashboard" />
             ) : !tiles || tiles.length === 0 ? (
                 <EmptyDashboardComponent loading={itemsLoading} canEdit={canEditDashboard} />

frontend/src/scenes/dashboard/Dashboards.stories.tsx

@@ -21,6 +21,7 @@ const meta: Meta = {
                 '/api/projects/:team_id/dashboards/': require('./__mocks__/dashboards.json'),
                 '/api/projects/:team_id/dashboards/1/': require('./__mocks__/dashboard1.json'),
                 '/api/projects/:team_id/dashboards/1/collaborators/': [],
+                '/api/projects/:team_id/dashboards/2/': [500, { detail: 'Server error' }],
                 '/api/projects/:team_id/dashboard_templates/': require('./__mocks__/dashboard_templates.json'),
                 '/api/projects/:team_id/dashboard_templates/json_schema/': require('./__mocks__/dashboard_template_schema.json'),
                 '/api/projects/:team_id/dashboards/:dash_id/sharing/': {
@@ -129,3 +130,17 @@ export const Edit = (): JSX.Element => {
     }, [])
     return <App />
 }
+
+export const NotFound = (): JSX.Element => {
+    useEffect(() => {
+        router.actions.push(urls.dashboard(1000))
+    }, [])
+    return <App />
+}
+
+export const Erroring = (): JSX.Element => {
+    useEffect(() => {
+        router.actions.push(urls.dashboard(2))
+    })
+    return <App />
+}

frontend/src/scenes/dashboard/dashboardLogic.queryCancellation.test.ts

@@ -4,6 +4,7 @@ import { MOCK_TEAM_ID } from 'lib/api.mock'
 import { now } from 'lib/dayjs'
 import { eventUsageLogic } from 'lib/utils/eventUsageLogic'
 import { dashboardLogic } from 'scenes/dashboard/dashboardLogic'
+// FIXME: Importing a .test.ts file causes all tests within it to be ran again as part of THIS file
 import { boxToString, dashboardResult, insightOnDashboard, tileFromInsight } from 'scenes/dashboard/dashboardLogic.test'
 
 import { useMocks } from '~/mocks/jest'

frontend/src/scenes/dashboard/dashboardLogic.test.ts

@@ -441,7 +441,7 @@ describe('dashboardLogic', () => {
 
         it('allows consumers to respond', async () => {
             await expectLogic(logic).toFinishAllListeners().toMatchValues({
-                receivedErrorsFromAPI: true,
+                dashboardFailedToLoad: true,
             })
         })
     })
@@ -478,20 +478,20 @@ describe('dashboardLogic', () => {
 
             it('fetches dashboard items on mount', async () => {
                 await expectLogic(logic)
-                    .toDispatchActions(['loadDashboardItems'])
+                    .toDispatchActions(['loadDashboard'])
                     .toMatchValues({
                         dashboard: null,
                         tiles: [],
                         insightTiles: [],
                         textTiles: [],
                     })
-                    .toDispatchActions(['loadDashboardItemsSuccess'])
+                    .toDispatchActions(['loadDashboardSuccess'])
                     .toMatchValues({
                         dashboard: expect.objectContaining(dashboards['5']),
                         tiles: truth((tiles) => tiles.length === 3),
                         insightTiles: truth((insightTiles) => insightTiles.length === 2),
                         textTiles: truth((textTiles) => textTiles.length === 1),
-                        receivedErrorsFromAPI: false,
+                        dashboardFailedToLoad: false,
                     })
             })
         })
@@ -675,7 +675,7 @@ describe('dashboardLogic', () => {
                 } as InsightModel)
             })
                 .toFinishAllListeners()
-                .toDispatchActions(['loadDashboardItems'])
+                .toDispatchActions(['loadDashboard'])
         })
     })
 
@@ -688,7 +688,7 @@ describe('dashboardLogic', () => {
         it('fetches dashboard items on mount', async () => {
             await expectLogic(logic)
                 .toFinishAllListeners()
-                .toDispatchActions(['loadDashboardItemsSuccess'])
+                .toDispatchActions(['loadDashboardSuccess'])
                 .toMatchValues({
                     dashboard: truth(
                         ({ tiles }) => tiles.filter((i: DashboardTile) => i.insight?.result === null).length === 2
@@ -747,7 +747,7 @@ describe('dashboardLogic', () => {
             logic = dashboardLogic({ id: 9 })
             logic.mount()
             await expectLogic(logic)
-                .toDispatchActions(['loadDashboardItemsSuccess'])
+                .toDispatchActions(['loadDashboardSuccess'])
                 .toNotHaveDispatchedActions(['refreshAllDashboardItems'])
                 .toFinishListeners()
         })
@@ -818,6 +818,6 @@ describe('dashboardLogic', () => {
             }))
         ).toEqual([])
         // Ensuring we do go back to the API for 800, which was added to dashboard 5
-        expectLogic(fiveLogic).toDispatchActions(['loadDashboardItems']).toFinishAllListeners()
+        expectLogic(fiveLogic).toDispatchActions(['loadDashboard']).toFinishAllListeners()
     })
 })

frontend/src/scenes/dashboard/dashboardLogic.tsx

@@ -142,7 +142,7 @@ export const dashboardLogic = kea<dashboardLogicType>([
     }),
 
     actions({
-        loadDashboardItems: (payload: { refresh?: boolean; action: string }) => payload,
+        loadDashboard: (payload: { refresh?: boolean; action: string }) => payload,
         triggerDashboardUpdate: (payload) => ({ payload }),
         /** The current state in which the dashboard is being viewed, see DashboardMode. */
         setDashboardMode: (mode: DashboardMode | null, source: DashboardEventSource | null) => ({ mode, source }),
@@ -201,12 +201,11 @@ export const dashboardLogic = kea<dashboardLogicType>([
         dashboard: [
             null as DashboardType | null,
             {
-                loadDashboardItems: async ({ refresh, action }) => {
+                loadDashboard: async ({ refresh, action }) => {
                     const dashboardQueryId = uuid()
                     actions.loadingDashboardItemsStarted(action, dashboardQueryId)
 
                     try {
-                        // :TODO: Send dashboardQueryId forward as well if refreshing
                         const apiUrl = values.apiUrl(refresh)
                         const dashboardResponse: Response = await api.getResponse(apiUrl)
                         const dashboard: DashboardType = await getJSONOrNull(dashboardResponse)
@@ -298,12 +297,11 @@ export const dashboardLogic = kea<dashboardLogicType>([
         ],
     })),
     reducers(({ props }) => ({
-        receivedErrorsFromAPI: [
+        dashboardFailedToLoad: [
             false,
             {
-                loadDashboardItemsSuccess: () => false,
-                loadDashboardItemsFailure: () => true,
-                abortQuery: () => false,
+                loadDashboardSuccess: () => false,
+                loadDashboardFailure: () => true,
             },
         ],
         filters: [
@@ -318,12 +316,15 @@ export const dashboardLogic = kea<dashboardLogicType>([
                     ...state,
                     properties: properties || null,
                 }),
-                loadDashboardItemsSuccess: (state, { dashboard }) => ({
-                    ...state,
-                    date_from: dashboard?.filters.date_from || null,
-                    date_to: dashboard?.filters.date_to || null,
-                    properties: dashboard?.filters.properties || [],
-                }),
+                loadDashboardSuccess: (state, { dashboard }) =>
+                    dashboard
+                        ? {
+                              ...state,
+                              date_from: dashboard?.filters.date_from || null,
+                              date_to: dashboard?.filters.date_to || null,
+                              properties: dashboard?.filters.properties || [],
+                          }
+                        : state,
             },
         ],
         dashboard: [
@@ -433,7 +434,7 @@ export const dashboardLogic = kea<dashboardLogicType>([
                 },
             },
         ],
-        loadTimer: [null as Date | null, { loadDashboardItems: () => new Date() }],
+        loadTimer: [null as Date | null, { loadDashboard: () => new Date() }],
         dashboardLoadTimerData: [
             { dashboardQueryId: '', action: '', startTime: 0, responseBytes: 0 },
             {
@@ -513,7 +514,7 @@ export const dashboardLogic = kea<dashboardLogicType>([
         shouldReportOnAPILoad: [
             /* Whether to report viewed/analyzed events after the API is loaded (and this logic is mounted).
             We need this because the DashboardView component might be mounted (and subsequent `useEffect`) before the API request
-            to `loadDashboardItems` is completed (e.g. if you open PH directly to a dashboard)
+            to `loadDashboard` is completed (e.g. if you open PH directly to a dashboard)
             */
             false,
             {
@@ -686,16 +687,22 @@ export const dashboardLogic = kea<dashboardLogicType>([
             },
         ],
         breadcrumbs: [
-            (s) => [s.dashboard],
-            (dashboard): Breadcrumb[] => [
+            (s) => [s.dashboard, s.dashboardLoading, s.dashboardFailedToLoad],
+            (dashboard, dashboardLoading, dashboardFailedToLoad): Breadcrumb[] => [
                 {
                     key: Scene.Dashboards,
                     name: 'Dashboards',
                     path: urls.dashboards(),
                 },
                 {
                     key: [Scene.Dashboard, dashboard?.id || 'new'],
-                    name: dashboard?.id ? dashboard.name || 'Unnamed' : null,
+                    name: dashboard?.id
+                        ? dashboard.name
+                        : dashboardFailedToLoad
+                        ? 'Could not load'
+                        : !dashboardLoading
+                        ? 'Not found'
+                        : null,
                     onRename: async (name) => {
                         if (dashboard) {
                             await dashboardsModel.asyncActions.updateDashboard({
@@ -732,10 +739,10 @@ export const dashboardLogic = kea<dashboardLogicType>([
             if (props.id) {
                 if (props.dashboard) {
                     // If we already have dashboard data, use it. Should the data turn out to be stale,
-                    // the loadDashboardItemsSuccess listener will initiate a refresh
-                    actions.loadDashboardItemsSuccess(props.dashboard)
+                    // the loadDashboardSuccess listener will initiate a refresh
+                    actions.loadDashboardSuccess(props.dashboard)
                 } else {
-                    actions.loadDashboardItems({
+                    actions.loadDashboard({
                         refresh: false,
                         action: 'initial_load',
                     })
@@ -772,17 +779,17 @@ export const dashboardLogic = kea<dashboardLogicType>([
         setRefreshError: sharedListeners.reportRefreshTiming,
         setRefreshStatuses: sharedListeners.reportRefreshTiming,
         setRefreshStatus: sharedListeners.reportRefreshTiming,
-        loadDashboardItemsFailure: sharedListeners.reportLoadTiming,
+        loadDashboardFailure: sharedListeners.reportLoadTiming,
         [insightsModel.actionTypes.duplicateInsightSuccess]: () => {
             // TODO this is a bit hacky, but we need to reload the dashboard to get the new insight
             // TODO when duplicated from a dashboard we should carry the context so only one logic needs to reload
             // TODO or we should duplicate the tile (and implicitly the insight)
-            actions.loadDashboardItems({ action: 'update' })
+            actions.loadDashboard({ action: 'update' })
         },
         [dashboardsModel.actionTypes.tileAddedToDashboard]: ({ dashboardId }) => {
             // when adding an insight to a dashboard, we need to reload the dashboard to get the new insight
             if (dashboardId === props.id) {
-                actions.loadDashboardItems({ action: 'update' })
+                actions.loadDashboard({ action: 'update' })
             }
         },
         [dashboardsModel.actionTypes.updateDashboardInsight]: ({ insight, extraDashboardIds }) => {
@@ -796,7 +803,7 @@ export const dashboardLogic = kea<dashboardLogicType>([
 
             if (tileIndex === -1) {
                 // this is a new tile created from an insight context we need to reload the dashboard
-                actions.loadDashboardItems({ action: 'update' })
+                actions.loadDashboard({ action: 'update' })
             }
         },
         updateLayouts: () => {
@@ -1026,10 +1033,14 @@ export const dashboardLogic = kea<dashboardLogicType>([
                 }, values.autoRefresh.interval * 1000)
             }
         },
-        loadDashboardItemsSuccess: function (...args) {
+        loadDashboardSuccess: function (...args) {
             void sharedListeners.reportLoadTiming(...args)
 
-            const dashboard = values.dashboard as DashboardType
+            if (!values.dashboard) {
+                return // We hit a 404
+            }
+
+            const dashboard = values.dashboard
             const { action, dashboardQueryId, startTime, responseBytes } = values.dashboardLoadTimerData
             const lastRefresh = sortDates(dashboard.tiles.map((tile) => tile.last_refresh))
 

frontend/src/scenes/feature-flags/FeatureFlag.tsx

@@ -618,21 +618,19 @@ function UsageTab({ featureFlag }: { id: string; featureFlag: FeatureFlagType })
     const { generateUsageDashboard, enrichUsageDashboard } = useActions(featureFlagLogic)
     const { featureFlagLoading } = useValues(featureFlagLogic)
     let dashboard: DashboardType | null = null
-    let connectedDashboardExists = false
     if (dashboardId) {
+        // FIXME: Refactor out into <ConnectedDashboard />, as React hooks under conditional branches are no good
         const dashboardLogicValues = useValues(
             dashboardLogic({ id: dashboardId, placement: DashboardPlacement.FeatureFlag })
         )
         dashboard = dashboardLogicValues.dashboard
-        connectedDashboardExists = !dashboardLogicValues.receivedErrorsFromAPI
     }
 
     const { closeEnrichAnalyticsNotice } = useActions(featureFlagsLogic)
     const { enrichAnalyticsNoticeAcknowledged } = useValues(featureFlagsLogic)
 
     useEffect(() => {
         if (
-            connectedDashboardExists &&
             dashboard &&
             hasEnrichedAnalytics &&
             !(dashboard.tiles?.find((tile) => (tile.insight?.name?.indexOf('Feature Viewed') ?? -1) > -1) !== undefined)
@@ -652,7 +650,7 @@ function UsageTab({ featureFlag }: { id: string; featureFlag: FeatureFlagType })
 
     return (
         <div>
-            {connectedDashboardExists ? (
+            {dashboard ? (
                 <>
                     {!hasEnrichedAnalytics && !enrichAnalyticsNoticeAcknowledged && (
                         <LemonBanner type="info" className="mb-3" onClose={() => closeEnrichAnalyticsNotice()}>

frontend/src/scenes/insights/insightLogic.test.ts

@@ -633,7 +633,7 @@ describe('insightLogic', () => {
             // 1. the dashboard is mounted
             const dashLogic = dashboardLogic({ id: 33 })
             dashLogic.mount()
-            await expectLogic(dashLogic).toDispatchActions(['loadDashboardItemsSuccess'])
+            await expectLogic(dashLogic).toDispatchActions(['loadDashboardSuccess'])
 
             // 2. mount the insight
             logic = insightLogic({ dashboardItemId: Insight42, dashboardId: 33 })
@@ -651,7 +651,7 @@ describe('insightLogic', () => {
             // 1. the dashboard is mounted
             const dashLogic = dashboardLogic({ id: 33 })
             dashLogic.mount()
-            await expectLogic(dashLogic).toDispatchActions(['loadDashboardItemsSuccess'])
+            await expectLogic(dashLogic).toDispatchActions(['loadDashboardSuccess'])
 
             // 2. mount the insight
             logic = insightLogic({ dashboardItemId: Insight42, dashboardId: 1 })