Merge pull request #5523 from metabase/fix-3347

Add non-blocking "Adding..." UI state during db creation

frontend/src/metabase/__support__/integrated_tests.js

@@ -140,7 +140,6 @@ if (process.env.E2E_HOST) {
  *     * getting a React container subtree for the current route
  */
 
-// Todo: Add a safeguard against not waiting createTestStore to finish
 export const createTestStore = async () => {
     hasFinishedCreatingStore = false;
     hasStartedCreatingStore = true;
@@ -222,8 +221,8 @@ const testStoreEnhancer = (createStore, history) => {
                 }
             },
 
-            getDispatchedActions: () => {
-                return store._dispatchedActions;
+            logDispatchedActions: () => {
+                console.log(`Dispatched actions so far: ${store._dispatchedActions.map((a) => a.type).join(", ")}`);
             },
 
             pushPath: (path) => history.push(path),

frontend/src/metabase/admin/databases/containers/DatabaseListApp.integ.spec.js

 import {
     login,
-    createTestStore,
+    createTestStore, clickRouterLink,
 } from "metabase/__support__/integrated_tests";
 
 import { mount } from "enzyme";
-import { FETCH_DATABASES, DELETE_DATABASE } from "metabase/admin/databases/database"
+import {
+    FETCH_DATABASES, DELETE_DATABASE, SAVE_DATABASE, INITIALIZE_DATABASE,
+    START_ADD_DATABASE
+} from "metabase/admin/databases/database"
 import DatabaseListApp from "metabase/admin/databases/containers/DatabaseListApp";
-import { delay } from "metabase/lib/promise"
 
 import { MetabaseApi } from 'metabase/services'
+import DatabaseEditApp from "metabase/admin/databases/containers/DatabaseEditApp";
 
 describe('dashboard list', () => {
 
@@ -29,10 +32,60 @@ describe('dashboard list', () => {
 
     })
 
+    describe('adds', () => {
+        it('should not block adding a new db', async () => {
+            MetabaseApi.db_create = async (db) => { return {...db, id: 10}; };
+
+            const store = await createTestStore()
+            store.pushPath("/admin/databases");
+
+            const app = mount(store.getAppContainer())
+            await store.waitForActions([FETCH_DATABASES])
+
+            const listAppBeforeAdd = app.find(DatabaseListApp)
+
+            const addDbButton = listAppBeforeAdd.find('.Button.Button--primary').first()
+            clickRouterLink(addDbButton)
+
+            const dbDetailsForm = app.find(DatabaseEditApp);
+            expect(dbDetailsForm.length).toBe(1);
+
+            await store.waitForActions([INITIALIZE_DATABASE]);
+
+            expect(dbDetailsForm.find('button[children="Save"]').props().disabled).toBe(true)
+
+            const updateInputValue = (name, value) =>
+                dbDetailsForm.find(`input[name="${name}"]`).simulate('change', { target: { value } });
+
+            updateInputValue("name", "Test db name");
+            updateInputValue("dbname", "test_postgres_db");
+            updateInputValue("user", "uberadmin");
+
+            expect(dbDetailsForm.find('button[children="Save"]').props().disabled).toBe(false)
+            dbDetailsForm.find('button[children="Save"]').simulate("submit");
+
+            await store.waitForActions([START_ADD_DATABASE])
+
+            expect(store.getPath()).toEqual("/admin/databases")
+
+            const listAppAfterAdd = app.find(DatabaseListApp)
+            expect(listAppAfterAdd.length).toBe(1);
+
+            // we should now have a disabled db row during the add
+            expect(listAppAfterAdd.find('tr.disabled').length).toEqual(1)
+
+            // wait until db creation finishes
+            await store.waitForActions([SAVE_DATABASE])
+
+            // there should be no disabled db rows now
+            expect(listAppAfterAdd.find('tr.disabled').length).toEqual(0)
+        })
+    })
+
     describe('deletes', () => {
         it('should not block deletes', async () => {
             // mock the db_delete method call to simulate a longer running delete
-            MetabaseApi.db_delete = () => delay(5000)
+            MetabaseApi.db_delete = () => {}
 
             const store = await createTestStore()
             store.pushPath("/admin/databases");

frontend/src/metabase/admin/databases/containers/DatabaseListApp.jsx

@@ -24,6 +24,7 @@ const mapStateToProps = (state, props) => {
         databases:            getDatabasesSorted(state),
         hasSampleDataset:     hasSampleDataset(state),
         engines:              MetabaseSettings.get('engines'),
+        adds:                 state.admin.databases.adds,
         deletes:              state.admin.databases.deletes
     }
 }
@@ -44,6 +45,12 @@ export default class DatabaseList extends Component {
         this.props.fetchDatabases();
     }
 
+    componentWillReceiveProps(newProps) {
+        if (!this.props.created && newProps.created) {
+            this.refs.createdDatabaseModal.open()
+        }
+    }
+
     render() {
         let { databases, hasSampleDataset, created, engines } = this.props;
 
@@ -63,8 +70,24 @@ export default class DatabaseList extends Component {
                             </tr>
                         </thead>
                         <tbody>
+                            { this.props.adds.map((database) =>
+                                <tr
+                                    key={database.name}
+                                    className={'disabled'}
+                                >
+                                    <td>
+                                        <Link to={"/admin/databases/" + database.id} className="text-bold link">
+                                            {database.name}
+                                        </Link>
+                                    </td>
+                                    <td>
+                                        {engines && engines[database.engine] ? engines[database.engine]['driver-name'] : database.engine}
+                                    </td>
+                                    <td className="text-right">Adding...</td>
+                                </tr>
+                            ) }
                             { databases ?
-                                databases.map(database => {
+                                [ databases.map(database => {
                                     const isDeleting = this.props.deletes.indexOf(database.id) !== -1
                                     return (
                                         <tr
@@ -98,7 +121,8 @@ export default class DatabaseList extends Component {
                                                 )
                                             }
                                         </tr>
-                                    )})
+                                    )}),
+                                ]
                             :
                                 <tr>
                                     <td colSpan={4}>

frontend/src/metabase/admin/databases/database.js

@@ -12,9 +12,9 @@ import { MetabaseApi } from "metabase/services";
 const RESET = "metabase/admin/databases/RESET";
 const SELECT_ENGINE = "metabase/admin/databases/SELECT_ENGINE";
 export const FETCH_DATABASES = "metabase/admin/databases/FETCH_DATABASES";
-const INITIALIZE_DATABASE = "metabase/admin/databases/INITIALIZE_DATABASE";
+export const INITIALIZE_DATABASE = "metabase/admin/databases/INITIALIZE_DATABASE";
 const ADD_SAMPLE_DATASET = "metabase/admin/databases/ADD_SAMPLE_DATASET";
-const SAVE_DATABASE = "metabase/admin/databases/SAVE_DATABASE";
+export const SAVE_DATABASE = "metabase/admin/databases/SAVE_DATABASE";
 export const DELETE_DATABASE = "metabase/admin/databases/DELETE_DATABASE";
 const SYNC_DATABASE = "metabase/admin/databases/SYNC_DATABASE";
 
@@ -73,6 +73,9 @@ export const addSampleDataset = createThunkAction(ADD_SAMPLE_DATASET, function()
     };
 });
 
+export const START_ADD_DATABASE = 'metabase/admin/databases/START_ADD_DATABASE'
+const startAddDatabase = createAction(START_ADD_DATABASE)
+
 // saveDatabase
 export const saveDatabase = createThunkAction(SAVE_DATABASE, function(database, details) {
     return async function(dispatch, getState) {
@@ -88,8 +91,14 @@ export const saveDatabase = createThunkAction(SAVE_DATABASE, function(database,
             } else {
                 //$scope.$broadcast("form:api-success", "Successfully created!");
                 //$scope.$emit("database:created", new_database);
+                dispatch(push('/admin/databases'))
+                dispatch(startAddDatabase(database))
                 savedDatabase = await MetabaseApi.db_create(database);
                 MetabaseAnalytics.trackEvent("Databases", "Create", database.engine);
+
+                // update the db metadata already here because otherwise there will be a gap between "Adding..." status
+                // and seeing the db that was just added
+                await dispatch(fetchDatabases())
                 dispatch(push('/admin/databases?created='+savedDatabase.id));
             }
 
@@ -110,15 +119,15 @@ export const saveDatabase = createThunkAction(SAVE_DATABASE, function(database,
     };
 });
 
-const START_DELETE = 'metabase/admin/databases/START_DELETE'
-const startDelete = createAction(START_DELETE)
+const START_DELETE_DATABASE = 'metabase/admin/databases/START_DELETE_DATABASE'
+const startDeleteDatabase = createAction(START_DELETE_DATABASE)
 
 
 // deleteDatabase
 export const deleteDatabase = createThunkAction(DELETE_DATABASE, function(databaseId, redirect=true) {
     return async function(dispatch, getState) {
         try {
-            dispatch(startDelete(databaseId))
+            dispatch(startDeleteDatabase(databaseId))
             dispatch(push('/admin/databases/'));
             await MetabaseApi.db_delete({"dbId": databaseId});
             MetabaseAnalytics.trackEvent("Databases", "Delete", redirect ? "Using Detail" : "Using List");
@@ -159,12 +168,21 @@ const editingDatabase = handleActions({
     [SELECT_ENGINE]: { next: (state, { payload }) => ({...state, engine: payload }) }
 }, null);
 
+const adds = handleActions({
+    [START_ADD_DATABASE]: {
+        next: (state, { payload }) => state.concat([payload])
+    },
+    [SAVE_DATABASE]: {
+        next: (state, { payload }) => state.filter((db) => db.name !== payload.database.name)
+    }
+}, []);
+
 const deletes = handleActions({
-    [START_DELETE]: {
+    [START_DELETE_DATABASE]: {
         next: (state, { payload }) => state.concat([payload])
     },
     [DELETE_DATABASE]: {
-        next: (state, { payload }) => state.splice(state.indexOf(payload), 1)
+        next: (state, { payload }) => state.filter((dbId) => dbId !== payload)
     }
 }, []);
 
@@ -178,5 +196,6 @@ export default combineReducers({
     databases,
     editingDatabase,
     formState,
+    adds,
     deletes
 });

frontend/src/metabase/store.js

@@ -24,11 +24,6 @@ const thunkWithDispatchAction = ({ dispatch, getState }) => next => action => {
     return next(action);
 };
 
-let middleware = [thunkWithDispatchAction, promise];
-if (DEBUG) {
-    middleware.push(logger);
-}
-
 const devToolsExtension = window.devToolsExtension ? window.devToolsExtension() : (f => f);
 
 export function getStore(reducers, history, intialState, enhancer = (a) => a) {
@@ -39,7 +34,12 @@ export function getStore(reducers, history, intialState, enhancer = (a) => a) {
         routing,
     });
 
-    middleware.push(routerMiddleware(history));
+    const middleware = [
+        thunkWithDispatchAction,
+        promise,
+        ...(DEBUG ? [logger] : []),
+        routerMiddleware(history)
+    ];
 
     return createStore(reducer, intialState, compose(
         applyMiddleware(...middleware),