Improved Dashboard Error Feedback (#7427)

* make loading state more obvious * error state if the API fails * better handle errors in whole dashboard or individual dashboard items * take error knowledge out of the empty state component * improved name for skipping insight error details * make a props item optional * Update frontend/src/styles/global.scss Co-authored-by: Michael Matloka <dev@twixes.com> * dashboard item error state matches container * make background transparent, not spinner Co-authored-by: Michael Matloka <dev@twixes.com>
PostHog · Dec 1, 2021 · df88b7d · df88b7d
1 parent a64b808
commit df88b7d
Show file tree

Hide file tree

Showing 10 changed files with 191 additions and 84 deletions.
diff --git a/frontend/src/scenes/dashboard/Dashboard.tsx b/frontend/src/scenes/dashboard/Dashboard.tsx
@@ -16,6 +16,7 @@ import { EmptyDashboardComponent } from './EmptyDashboardComponent'
 import { NotFound } from 'lib/components/NotFound'
 import { DashboardReloadAction, LastRefreshText } from 'scenes/dashboard/DashboardReloadAction'
 import { SceneExport } from 'scenes/sceneTypes'
+import { InsightErrorState } from 'scenes/insights/EmptyStates'
 
 interface Props {
     id?: string
@@ -44,6 +45,7 @@ function DashboardView(): JSX.Element {
         items,
         filters: dashboardFilters,
         dashboardMode,
+        receivedErrorsFromAPI,
     } = useValues(dashboardLogic)
     const { dashboardsLoading } = useValues(dashboardsModel)
     const { setDashboardMode, addGraph, setDates } = useActions(dashboardLogic)
@@ -100,7 +102,11 @@ function DashboardView(): JSX.Element {
     return (
         <div className="dashboard">
             {dashboardMode !== DashboardMode.Public && dashboardMode !== DashboardMode.Internal && <DashboardHeader />}
-            {items && items.length ? (
+            {receivedErrorsFromAPI ? (
+                <InsightErrorState title={'There was an error loading this dashboard'} />
+            ) : !items || items.length === 0 ? (
+                <EmptyDashboardComponent />
+            ) : (
                 <div>
                     <div className="dashboard-items-actions">
                         <div className="left-item">
@@ -135,8 +141,6 @@ function DashboardView(): JSX.Element {
                     </div>
                     <DashboardItems />
                 </div>
-            ) : (
-                <EmptyDashboardComponent />
             )}
         </div>
     )

diff --git a/frontend/src/scenes/dashboard/DashboardItem.tsx b/frontend/src/scenes/dashboard/DashboardItem.tsx
@@ -51,6 +51,7 @@ import { urls } from 'scenes/urls'
 interface DashboardItemProps {
     item: DashboardItemType
     dashboardId?: number
+    receivedErrorFromAPI?: boolean
     updateItemColor?: (insightId: number, itemClassName: string) => void
     setDiveDashboard?: (insightId: number, diveDashboard: number | null) => void
     loadDashboardItems?: () => void
@@ -148,6 +149,7 @@ const dashboardDiveLink = (dive_dashboard: number, dive_source_id: InsightShortI
 export function DashboardItem({
     item,
     dashboardId,
+    receivedErrorFromAPI,
     updateItemColor,
     setDiveDashboard,
     loadDashboardItems,
@@ -246,8 +248,8 @@ export function DashboardItem({
         }
 
         // Insight agnostic empty states
-        if (showErrorMessage) {
-            return <InsightErrorState />
+        if (showErrorMessage || receivedErrorFromAPI) {
+            return <InsightErrorState excludeDetail={true} />
         }
         if (showTimeoutMessage) {
             return <InsightTimeoutState isLoading={isLoading} />
@@ -258,7 +260,7 @@ export function DashboardItem({
 
     // Empty states that can coexist with the graph (e.g. Loading)
     const CoexistingEmptyState = (() => {
-        if (isLoading || insightLoading) {
+        if (isLoading || insightLoading || isReloading) {
             return <Loading />
         }
         return null
@@ -272,8 +274,9 @@ export function DashboardItem({
             } ph-no-capture`}
             {...longPressProps}
             data-attr={'dashboard-item-' + index}
-            style={{ border: isHighlighted ? '1px solid var(--primary)' : undefined, opacity: isReloading ? 0.5 : 1 }}
+            style={{ border: isHighlighted ? '1px solid var(--primary)' : undefined }}
         >
+            {!BlockingEmptyState && CoexistingEmptyState}
             <div className={`dashboard-item-container ${className}`}>
                 <div className="dashboard-item-header" style={{ cursor: isOnEditMode ? 'move' : 'inherit' }}>
                     <div className="dashboard-item-title" data-attr="dashboard-item-title">
@@ -570,7 +573,6 @@ export function DashboardItem({
                 )}
 
                 <div className={`dashboard-item-content ${_type}`} onClickCapture={onClick}>
-                    {!BlockingEmptyState && CoexistingEmptyState}
                     {!!BlockingEmptyState ? (
                         BlockingEmptyState
                     ) : (

diff --git a/frontend/src/scenes/dashboard/DashboardItems.scss b/frontend/src/scenes/dashboard/DashboardItems.scss
@@ -67,10 +67,6 @@
 
     // sync this with lib/colors.js
 
-    .loading-overlay {
-        background: transparent;
-    }
-
     &.blue {
         --item-background: hsl(212, 63%, 40%);
         --item-darker: hsl(212, 63%, 35%);
@@ -107,9 +103,6 @@
     &.purple,
     &.green,
     &.black {
-        .loading-overlay {
-            filter: brightness(300%);
-        }
         color: white;
         background: var(--item-background);
         .dashboard-item-header .dashboard-item-title a {

diff --git a/frontend/src/scenes/dashboard/DashboardItems.tsx b/frontend/src/scenes/dashboard/DashboardItems.tsx
@@ -25,6 +25,7 @@ export function DashboardItems(): JSX.Element {
         dashboardMode,
         isRefreshing,
         highlightedInsightId,
+        refreshStatus,
     } = useValues(dashboardLogic)
     const {
         loadDashboardItems,
@@ -108,6 +109,7 @@ export function DashboardItems(): JSX.Element {
                     <DashboardItem
                         key={item.short_id}
                         doNotLoad
+                        receivedErrorFromAPI={refreshStatus[item.short_id]?.error || false}
                         dashboardId={dashboard?.id}
                         item={item}
                         layout={resizingItem?.i === item.short_id ? resizingItem : layoutForItem[item.short_id]}

diff --git a/frontend/src/scenes/dashboard/dashboardLogic.test.ts b/frontend/src/scenes/dashboard/dashboardLogic.test.ts
@@ -31,6 +31,15 @@ describe('dashboardLogic', () => {
                     { ...dashboardJson.items[1], id: 999, short_id: '999' },
                 ],
             }
+        } else if (pathname === `api/projects/${MOCK_TEAM_ID}/dashboards/7/`) {
+            throw new Error('💣')
+        } else if (pathname === `api/projects/${MOCK_TEAM_ID}/dashboards/8/`) {
+            return {
+                ...dashboardJson,
+                items: [{ id: 1001, short_id: '1001' }],
+            }
+        } else if (pathname === `api/projects/${MOCK_TEAM_ID}/insights/1001`) {
+            throw new Error('💣')
         } else if (pathname.startsWith(`api/projects/${MOCK_TEAM_ID}/insights/`)) {
             return dashboardJson.items.find(({ id }: any) => String(id) === pathname.split('/')[4])
         }
@@ -51,6 +60,43 @@ describe('dashboardLogic', () => {
         })
     })
 
+    describe('when the dashboard API errors', () => {
+        initKeaTestLogic({
+            logic: dashboardLogic,
+            props: {
+                id: 7,
+            },
+            onLogic: (l) => (logic = l),
+        })
+
+        it('allows consumers to respond', async () => {
+            await expectLogic(logic).toMatchValues({
+                receivedErrorsFromAPI: true,
+            })
+        })
+    })
+
+    describe('when a dashboard item API errors', () => {
+        initKeaTestLogic({
+            logic: dashboardLogic,
+            props: {
+                id: 8,
+            },
+            onLogic: (l) => (logic = l),
+        })
+
+        it('allows consumers to respond', async () => {
+            await expectLogic(logic, () => {
+                // try and load dashboard items data once dashboard is loaded
+                logic.actions.refreshAllDashboardItemsManual()
+            })
+                .toFinishAllListeners()
+                .toMatchValues({
+                    refreshStatus: { 1001: { error: true } },
+                })
+        })
+    })
+
     describe('when props id is set to a number', () => {
         initKeaTestLogic({
             logic: dashboardLogic,
@@ -76,6 +122,7 @@ describe('dashboardLogic', () => {
                     .toMatchValues({
                         allItems: dashboardJson,
                         items: truth((items) => items.length === 2),
+                        receivedErrorsFromAPI: false,
                     })
             })
         })

diff --git a/frontend/src/scenes/dashboard/dashboardLogic.tsx b/frontend/src/scenes/dashboard/dashboardLogic.tsx
@@ -45,6 +45,9 @@ export const dashboardLogic = kea<dashboardLogicType<DashboardLogicProps>>({
     key: (props) => props.id || 'dashboardLogic',
 
     actions: {
+        setReceivedErrorsFromAPI: (receivedErrors: boolean) => ({
+            receivedErrors,
+        }),
         addNewDashboard: true,
         loadDashboardItems: ({
             refresh,
@@ -90,6 +93,8 @@ export const dashboardLogic = kea<dashboardLogicType<DashboardLogicProps>>({
             null as DashboardType | null,
             {
                 loadDashboardItems: async ({ refresh, dive_source_id }) => {
+                    actions.setReceivedErrorsFromAPI(false)
+
                     if (!props.id) {
                         console.warn('Called `loadDashboardItems` but ID is not set.')
                         return
@@ -111,6 +116,7 @@ export const dashboardLogic = kea<dashboardLogicType<DashboardLogicProps>>({
                         if (error.status === 404) {
                             return []
                         }
+                        actions.setReceivedErrorsFromAPI(true)
                         throw error
                     }
                 },
@@ -123,6 +129,13 @@ export const dashboardLogic = kea<dashboardLogicType<DashboardLogicProps>>({
         ],
     }),
     reducers: ({ props }) => ({
+        receivedErrorsFromAPI: [
+            false,
+            {
+                setReceivedErrorsFromAPI: (_: boolean, { receivedErrors }: { receivedErrors: boolean }) =>
+                    receivedErrors,
+            },
+        ],
         filters: [
             { date_from: null, date_to: null } as FilterType,
             {

diff --git a/frontend/src/scenes/insights/EmptyStates/EmptyStates.tsx b/frontend/src/scenes/insights/EmptyStates/EmptyStates.tsx
@@ -97,56 +97,63 @@ export function InsightTimeoutState({ isLoading }: { isLoading: boolean }): JSX.
     )
 }
 
-export function InsightErrorState(): JSX.Element {
+export interface InsightErrorStateProps {
+    excludeDetail?: boolean
+    title?: string
+}
+
+export function InsightErrorState({ excludeDetail, title }: InsightErrorStateProps): JSX.Element {
     return (
-        <div className="insight-empty-state error">
+        <div className={clsx(['insight-empty-state', 'error', { 'match-container': excludeDetail }])}>
             <div className="empty-state-inner">
                 <div className="illustration-main">
                     <IllustrationDanger />
                 </div>
-                <h2>There was an error completing this query</h2>
-                <div className="mt">
-                    We apologize for this unexpected situation. There are a few things you can do:
-                    <ol>
-                        <li>
-                            First and foremost you can <b>try again</b>. We recommended you wait a few moments before
-                            doing so.
-                        </li>
-                        <li>
-                            <a
-                                data-attr="insight-error-raise-issue"
-                                href="https://github.com/PostHog/posthog/issues/new?labels=bug&template=bug_report.md"
-                                target="_blank"
-                                rel="noreferrer noopener"
-                            >
-                                Raise an issue
-                            </a>{' '}
-                            in our GitHub repository.
-                        </li>
-                        <li>
-                            Get in touch with us{' '}
-                            <a
-                                data-attr="insight-error-slack"
-                                href="https://posthog.com/slack"
-                                rel="noopener noreferrer"
-                                target="_blank"
-                            >
-                                on Slack
-                            </a>
-                            .
-                        </li>
-                        <li>
-                            Email us at{' '}
-                            <a
-                                data-attr="insight-error-email"
-                                href="mailto:hey@posthog.com?subject=Insight%20graph%20error"
-                            >
-                                hey@posthog.com
-                            </a>
-                            .
-                        </li>
-                    </ol>
-                </div>
+                <h2>{title || 'There was an error completing this query'}</h2>
+                {!excludeDetail && (
+                    <div className="mt">
+                        We apologize for this unexpected situation. There are a few things you can do:
+                        <ol>
+                            <li>
+                                First and foremost you can <b>try again</b>. We recommended you wait a few moments
+                                before doing so.
+                            </li>
+                            <li>
+                                <a
+                                    data-attr="insight-error-raise-issue"
+                                    href="https://github.com/PostHog/posthog/issues/new?labels=bug&template=bug_report.md"
+                                    target="_blank"
+                                    rel="noreferrer noopener"
+                                >
+                                    Raise an issue
+                                </a>{' '}
+                                in our GitHub repository.
+                            </li>
+                            <li>
+                                Get in touch with us{' '}
+                                <a
+                                    data-attr="insight-error-slack"
+                                    href="https://posthog.com/slack"
+                                    rel="noopener noreferrer"
+                                    target="_blank"
+                                >
+                                    on Slack
+                                </a>
+                                .
+                            </li>
+                            <li>
+                                Email us at{' '}
+                                <a
+                                    data-attr="insight-error-email"
+                                    href="mailto:hey@posthog.com?subject=Insight%20graph%20error"
+                                >
+                                    hey@posthog.com
+                                </a>
+                                .
+                            </li>
+                        </ol>
+                    </div>
+                )}
             </div>
         </div>
     )

diff --git a/frontend/src/scenes/insights/Insight.scss b/frontend/src/scenes/insights/Insight.scss
@@ -314,6 +314,46 @@ $funnel_canvas_background: #fafafa;
     }
 }
 
+.dashboard-item {
+    .insight-empty-state {
+        &.match-container {
+            background-color: transparent;
+        }
+    }
+
+    &.purple {
+        .insight-empty-state {
+            &.match-container {
+                &.error {
+                    .illustration-main {
+                        color: $text_light;
+                    }
+
+                    h2 {
+                        color: $text_light;
+                    }
+                }
+            }
+        }
+    }
+
+    &:not(.purple) {
+        .insight-empty-state {
+            &.match-container {
+                &.error {
+                    .illustration-main {
+                        color: $danger;
+                    }
+
+                    h2 {
+                        color: $danger;
+                    }
+                }
+            }
+        }
+    }
+}
+
 .trends-insights-container {
     position: relative;
     min-height: min(calc(90vh - 16rem), 36rem);