Expand test coverage, fix bug with redirecting to QB if only Custom option available

frontend/src/metabase/new_query/containers/NewQueryOptions.jsx

@@ -85,7 +85,7 @@ export class NewQueryOptions extends Component {
 
     async componentWillMount() {
         this.props.resetQuery();
-        this.props.determineWhichOptionsToShow();
+        this.props.determineWhichOptionsToShow(this.getGuiQueryUrl);
     }
 
     getGuiQueryUrl = () => {
@@ -97,14 +97,9 @@ export class NewQueryOptions extends Component {
     }
 
     render() {
-<<<<<<< HEAD
-        const { isAdmin, metricSearchUrl } = this.props
-        const { loaded, hasDatabases, showMetricOption, showTableOption, showSQLOption } = this.state
-=======
-        const { isAdmin, metricSearchUrl, tableSearchUrl, newQueryOptions } = this.props
-        const { loaded, hasDatabases, showMetricOption, showTableOption, showSQLOption } = newQueryOptions
->>>>>>> 5dd4b14fe... Extract option showing logic to actions, refactor tests
-        const showCustomInsteadOfNewQuestionText = showMetricOption || showTableOption || isAdmin
+        const { isAdmin, metricSearchUrl, newQueryOptions } = this.props
+        const { loaded, hasDatabases, showMetricOption, showSQLOption } = newQueryOptions
+        const showCustomInsteadOfNewQuestionText = showMetricOption || isAdmin
 
         if (!loaded) {
             return <LoadingAndErrorWrapper loading={true}/>

frontend/src/metabase/new_query/new_query.js

@@ -44,7 +44,7 @@ const newQueryOptionsAllVisible = {
 
 export const DETERMINE_OPTIONS_STARTED = "metabase/new_query/DETERMINE_OPTIONS_STARTED"
 export const DETERMINE_OPTIONS = "metabase/new_query/DETERMINE_OPTIONS"
-export function determineWhichOptionsToShow() {
+export function determineWhichOptionsToShow(getGuiQueryUrl) {
     return async (dispatch, getState) => {
         // By default, show all options instantly to admins
         const isAdmin = getUserIsAdmin(getState())
@@ -82,7 +82,7 @@ export function determineWhichOptionsToShow() {
                 !showMetricOption && !showSQLOption && !showTableOption
 
             if (redirectToQueryBuilder) {
-                dispatch(push(this.getGuiQueryUrl()))
+                dispatch(push(getGuiQueryUrl()))
             } else {
                 return dispatch.action(DETERMINE_OPTIONS, {
                     loaded: true,

frontend/test/__support__/integrated_tests.js

@@ -68,6 +68,12 @@ export function useSharedNormalLogin() {
     warnAboutCreatingStoreBeforeLogin()
     loginSession = { id: process.env.TEST_FIXTURE_SHARED_NORMAL_LOGIN_SESSION_ID }
 }
+export const forBothAdminsAndNormalUsers = async (tests) => {
+    useSharedAdminLogin()
+    await tests()
+    useSharedNormalLogin()
+    await tests()
+}
 
 export function logout() {
     previousLoginSession = loginSession
@@ -360,6 +366,37 @@ export const waitForRequestToComplete = (method, urlRegex, { timeout = 5000 } =
     })
 }
 
+/**
+ * Lets you replace given API endpoints with mocked implementations for the lifetime of a test
+ */
+export async function withApiMocks(mocks, test) {
+    if (!mocks.every(([apiService, endpointName, mockMethod]) =>
+            _.isObject(apiService) && _.isString(endpointName) && _.isFunction(mockMethod)
+        )
+    ) {
+        throw new Error(
+            "Seems that you are calling \`withApiMocks\` with invalid parameters. " +
+            "The calls should be in format \`withApiMocks([[ApiService, endpointName, mockMethod], ...], tests)\`."
+        )
+    }
+
+    const originals = mocks.map(([apiService, endpointName]) => apiService[endpointName])
+
+    // Replace real API endpoints with mocks
+    mocks.forEach(([apiService, endpointName, mockMethod]) => {
+        apiService[endpointName] = mockMethod
+    })
+
+    try {
+        await test();
+    } finally {
+        // Restore original endpoints after tests, even in case of an exception
+        mocks.forEach(([apiService, endpointName], index) => {
+            apiService[endpointName] = originals[index]
+        })
+    }
+}
+
 // Patches the metabase/lib/api module so that all API queries contain the login credential cookie.
 // Needed because we are not in a real web browser environment.
 api._makeRequest = async (method, url, headers, requestBody, data, options) => {

frontend/test/query_builder/new_question.integ.spec.js

@@ -2,7 +2,7 @@ import { mount } from "enzyme"
 
 import {
     useSharedAdminLogin,
-    createTestStore, useSharedNormalLogin,
+    createTestStore, useSharedNormalLogin, forBothAdminsAndNormalUsers, withApiMocks, BROWSER_HISTORY_REPLACE,
 } from "__support__/integrated_tests";
 
 import EntitySearch, {
@@ -36,13 +36,14 @@ import {
     QUERY_COMPLETED,
 } from "metabase/query_builder/actions";
 
-import { MetricApi, SegmentApi } from "metabase/services";
+import { MetabaseApi, MetricApi, SegmentApi } from "metabase/services";
 import { SET_REQUEST_STATE } from "metabase/redux/requests";
 
 import StructuredQuery from "metabase-lib/lib/queries/StructuredQuery";
 
 import NativeQueryEditor from "metabase/query_builder/components/NativeQueryEditor";
 import NewQueryOption from "metabase/new_query/components/NewQueryOption";
+import NoDatabasesEmptyState from "metabase/reference/databases/NoDatabasesEmptyState";
 
 describe("new question flow", async () => {
     // test an instance with segments, metrics, etc as an admin
@@ -79,17 +80,92 @@ describe("new question flow", async () => {
             expect(store.getPath()).toBe("/question/new")
         })
 
-        it("renders normally on page load", async () => {
+        it("renders all options for both admins and normal users if metrics & segments exist", async () => {
+            await forBothAdminsAndNormalUsers(async () => {
+                const store = await createTestStore()
+
+                store.pushPath(Urls.newQuestion());
+                const app = mount(store.getAppContainer());
+                await store.waitForActions([DETERMINE_OPTIONS]);
+
+                expect(app.find(NewQueryOption).length).toBe(3)
+            })
+        });
+
+        it("does not show Metrics option for normal users if there are no metrics", async () => {
             useSharedNormalLogin()
-            const store = await createTestStore()
 
-            store.pushPath(Urls.newQuestion());
-            const app = mount(store.getAppContainer());
-            await store.waitForActions([DETERMINE_OPTIONS]);
+            await withApiMocks([
+                [MetricApi, "list", () => []],
+            ], async () => {
+                const store = await createTestStore()
+
+                store.pushPath(Urls.newQuestion());
+                const app = mount(store.getAppContainer());
+                await store.waitForActions([DETERMINE_OPTIONS]);
 
-            expect(app.find(NewQueryOption).length).toBe(3)
+                expect(app.find(NewQueryOption).filterWhere((c) => c.prop('title') === "Metrics").length).toBe(0)
+                expect(app.find(NewQueryOption).length).toBe(2)
+            })
         });
 
+        it("does not show SQL option for normal user if SQL write permissions are missing", async () => {
+            useSharedNormalLogin()
+
+            const disableWritePermissionsForDb = (db) => ({ ...db, native_permissions: "read" })
+            const realDbListWithTables = MetabaseApi.db_list_with_tables
+
+            await withApiMocks([
+                [MetabaseApi, "db_list_with_tables", async () =>
+                    (await realDbListWithTables()).map(disableWritePermissionsForDb)
+                ],
+            ], async () => {
+                const store = await createTestStore()
+
+                store.pushPath(Urls.newQuestion());
+                const app = mount(store.getAppContainer());
+                await store.waitForActions([DETERMINE_OPTIONS]);
+
+                expect(app.find(NewQueryOption).length).toBe(2)
+            })
+        })
+
+        it("redirects to query builder if there are no segments/metrics and no write sql permissions", async () => {
+            useSharedNormalLogin()
+
+            const disableWritePermissionsForDb = (db) => ({ ...db, native_permissions: "read" })
+            const realDbListWithTables = MetabaseApi.db_list_with_tables
+
+            await withApiMocks([
+                [MetricApi, "list", () => []],
+                [MetabaseApi, "db_list_with_tables", async () =>
+                    (await realDbListWithTables()).map(disableWritePermissionsForDb)
+                ],
+            ], async () => {
+                const store = await createTestStore()
+                store.pushPath(Urls.newQuestion());
+                mount(store.getAppContainer());
+                await store.waitForActions(BROWSER_HISTORY_REPLACE, INITIALIZE_QB);
+            })
+        })
+
+        it("shows an empty state if there are no databases", async () => {
+            await forBothAdminsAndNormalUsers(async () => {
+                await withApiMocks([
+                    [MetabaseApi, "db_list_with_tables", () => []]
+                ], async () => {
+                    const store = await createTestStore()
+
+                    store.pushPath(Urls.newQuestion());
+                    const app = mount(store.getAppContainer());
+                    await store.waitForActions([DETERMINE_OPTIONS]);
+
+                    expect(app.find(NewQueryOption).length).toBe(0)
+                    expect(app.find(NoDatabasesEmptyState).length).toBe(1)
+                })
+            })
+        })
+
         it("lets you start a custom gui question", async () => {
             useSharedNormalLogin()
             const store = await createTestStore()