From 296647fa1023e8a4844d52fdd03fc27b96845844 Mon Sep 17 00:00:00 2001
From: Marius Andra <marius.andra@gmail.com>
Date: Fri, 15 Oct 2021 12:45:52 +0200
Subject: [PATCH] Override filters if bad api result (#6470)

* dispatch loadResultsFailure if failed to load

* add tests for API failures

* don't mount dashboards model if it's not mounted
---
 .../src/scenes/insights/insightLogic.test.ts  | 63 ++++++++++++++++++-
 frontend/src/scenes/insights/insightLogic.tsx |  8 +--
 2 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/frontend/src/scenes/insights/insightLogic.test.ts b/frontend/src/scenes/insights/insightLogic.test.ts
index 42f8a8c76a5..d15ac2543ce 100644
--- a/frontend/src/scenes/insights/insightLogic.test.ts
+++ b/frontend/src/scenes/insights/insightLogic.test.ts
@@ -12,7 +12,10 @@ describe('insightLogic', () => {
     let logic: ReturnType<typeof insightLogic.build>
 
     mockAPI(async (url) => {
-        const { pathname } = url
+        const { pathname, searchParams } = url
+        const throwAPIError = (): void => {
+            throw { status: 0, statusText: 'error from the API' }
+        }
         if (['api/insight/42', 'api/insight/43'].includes(pathname)) {
             return {
                 result: pathname === 'api/insight/42' ? ['result from api'] : null,
@@ -23,9 +26,14 @@ describe('insightLogic', () => {
                     properties: [{ value: 'a', operator: PropertyOperator.Exact, key: 'a', type: 'a' }],
                 },
             }
+        } else if (['api/insight/44'].includes(pathname)) {
+            throwAPIError()
         } else if (
             ['api/insight', 'api/insight/session/', 'api/insight/trend/', 'api/insight/funnel/'].includes(pathname)
         ) {
+            if (searchParams?.events?.[0]?.throw) {
+                throwAPIError()
+            }
             return { result: ['result from api'] }
         }
         return defaultAPIMocks(url, { availableFeatures: [AvailableFeature.DASHBOARD_COLLABORATION] })
@@ -130,6 +138,36 @@ describe('insightLogic', () => {
             })
         })
 
+        describe('props with filters, no cached results, error from API', () => {
+            initKeaTestLogic({
+                logic: insightLogic,
+                props: {
+                    dashboardItemId: 42,
+                    cachedResults: undefined,
+                    filters: {
+                        insight: ViewType.TRENDS,
+                        events: [{ id: 3, throw: true }],
+                        properties: [{ value: 'a', operator: PropertyOperator.Exact, key: 'a', type: 'a' }],
+                    },
+                },
+                onLogic: (l) => (logic = l),
+            })
+
+            it('makes a query to load the results', async () => {
+                await expectLogic(logic)
+                    .toDispatchActions(['loadResults', 'loadResultsFailure'])
+                    .toMatchValues({
+                        insight: expect.objectContaining({ id: 42, result: null }),
+                        filters: expect.objectContaining({
+                            events: [expect.objectContaining({ id: 3 })],
+                            properties: [expect.objectContaining({ value: 'a' })],
+                        }),
+                    })
+                    .delay(1)
+                    .toNotHaveDispatchedActions(['loadResults', 'setFilters', 'updateInsight'])
+            })
+        })
+
         describe('props with no filters, no cached results, results from API', () => {
             initKeaTestLogic({
                 logic: insightLogic,
@@ -186,6 +224,29 @@ describe('insightLogic', () => {
                     })
             })
         })
+
+        describe('props with no filters, no cached results, API throws', () => {
+            initKeaTestLogic({
+                logic: insightLogic,
+                props: {
+                    dashboardItemId: 44, // 44 --> result: throws
+                    cachedResults: undefined,
+                    filters: undefined,
+                },
+                onLogic: (l) => (logic = l),
+            })
+
+            it('makes a query to load the results', async () => {
+                await expectLogic(logic)
+                    .toDispatchActions(['loadInsight', 'loadInsightFailure'])
+                    .toMatchValues({
+                        insight: expect.objectContaining({ id: 44, result: null, filters: {} }),
+                        filters: {},
+                    })
+                    .delay(1)
+                    .toNotHaveDispatchedActions(['loadResults', 'setFilters', 'updateInsight'])
+            })
+        })
     })
 
     describe('reacts to the URL', () => {
diff --git a/frontend/src/scenes/insights/insightLogic.tsx b/frontend/src/scenes/insights/insightLogic.tsx
index 69a82f3043f..3e79865398c 100644
--- a/frontend/src/scenes/insights/insightLogic.tsx
+++ b/frontend/src/scenes/insights/insightLogic.tsx
@@ -122,7 +122,7 @@ export const insightLogic = kea<insightLogicType>({
 
                     const dashboardItemId = props.dashboardItemId
                     actions.startQuery(queryId)
-                    if (dashboardItemId) {
+                    if (dashboardItemId && dashboardsModel.isMounted()) {
                         dashboardsModel.actions.updateDashboardRefreshStatus(dashboardItemId, true, null)
                     }
 
@@ -161,7 +161,7 @@ export const insightLogic = kea<insightLogicType>({
                         breakpoint()
                         cache.abortController = null
                         actions.endQuery(queryId, insight, null, e)
-                        if (dashboardItemId) {
+                        if (dashboardItemId && dashboardsModel.isMounted()) {
                             dashboardsModel.actions.updateDashboardRefreshStatus(dashboardItemId, false, null)
                         }
                         if (filters.insight === ViewType.FUNNELS) {
@@ -174,7 +174,7 @@ export const insightLogic = kea<insightLogicType>({
                                 e.message
                             )
                         }
-                        return values.insight
+                        throw e
                     }
                     breakpoint()
                     cache.abortController = null
@@ -183,7 +183,7 @@ export const insightLogic = kea<insightLogicType>({
                         (values.filters.insight as ViewType) || ViewType.TRENDS,
                         response.last_refresh
                     )
-                    if (dashboardItemId) {
+                    if (dashboardItemId && dashboardsModel.isMounted()) {
                         dashboardsModel.actions.updateDashboardRefreshStatus(
                             dashboardItemId,
                             false,