tidy up the DatabaseStep and add in full form validation support along with...

tidy up the DatabaseStep and add in full form validation support along with the ability to use the setup api to validate the connection details of possible new database creation.

resources/frontend_client/app/components/database/DatabaseDetailsForm.react.js

 "use strict";
 
 import React, { Component, PropTypes } from "react";
+import cx from "classnames";
 import _ from "underscore";
 
 import MetabaseCore from "metabase/lib/core";
@@ -9,22 +10,76 @@ import FormLabel from "metabase/components/form/FormLabel.react";
 import FormMessage from "metabase/components/form/FormMessage.react";
 
 
+// TODO - this should be somewhere more centralized
+function isEmpty(str) {
+    return (!str || 0 === str.length);
+}
+
+/**
+ * This is a form for capturing database details for a given `engine` supplied via props.
+ * The intention is to encapsulate the entire <form> with standard MB form styling and allow a callback
+ * function to receive the captured form input when the form is submitted.
+ */
 export default class DatabaseDetailsForm extends Component {
 
+    constructor(props) {
+        super(props);
+        this.state = { valid: false }
+    }
+
+    validateForm() {
+        let { engine } = this.props;
+        let { valid } = this.state;
+
+        let isValid = true;
+
+        // name is required
+        if (isEmpty(React.findDOMNode(this.refs.name).value)) isValid = false;
+
+        // go over individual fields
+        for (var fieldIdx in MetabaseCore.ENGINES[engine].fields) {
+            let field = MetabaseCore.ENGINES[engine].fields[fieldIdx],
+                ref = React.findDOMNode(this.refs[field.fieldName]);
+            if (ref && field.required && isEmpty(ref.value)) {
+                isValid = false;
+            }
+        }
+
+        if(isValid !== valid) {
+            this.setState({
+                'valid': isValid
+            });
+        }
+    }
+
+    onChange(fieldName) {
+        this.validateForm();
+    }
+
     formSubmitted(e) {
         e.preventDefault();
 
         let { engine, submitFn } = this.props;
 
-        // collect data && validate
+        // collect data
         let response = {
             'name': React.findDOMNode(this.refs.name).value
         };
+
         for (var fieldIdx in MetabaseCore.ENGINES[engine].fields) {
             let field = MetabaseCore.ENGINES[engine].fields[fieldIdx],
                 ref = React.findDOMNode(this.refs[field.fieldName]);
             if (ref) {
-                response[field.fieldName] = ref.value;
+                let val = (ref.value && ref.value !== "") ? ref.value : null;
+                if (val === null && field.placeholderIsDefault) {
+                    val = field.placeholder;
+                }
+
+                if (field.transform) {
+                    val = field.transform(val);
+                }
+
+                response[field.fieldName] = val;
             }
         }
 
@@ -33,6 +88,10 @@ export default class DatabaseDetailsForm extends Component {
     }
 
     renderFieldInput(field) {
+        let { details } = this.props;
+
+        let defaultValue = (details && field.fieldName in details) ? details[field.fieldName] : "";
+
         switch(field.type) {
             case 'select':
                 return (
@@ -50,25 +109,28 @@ export default class DatabaseDetailsForm extends Component {
 
             case 'password':
                 return (
-                    <input type="password" className="Form-input Form-offset full" ref={field.fieldName} name={field.fieldName} placeholder={field.placeholder} />
+                    <input type="password" className="Form-input Form-offset full" ref={field.fieldName} name={field.fieldName} defaultValue={defaultValue} placeholder={field.placeholder} onChange={this.onChange.bind(this, field.fieldName)} />
                );
 
             case 'text':
                 return (
-                    <input className="Form-input Form-offset full" ref={field.fieldName} name={field.fieldName} placeholder={field.placeholder} />
+                    <input className="Form-input Form-offset full" ref={field.fieldName} name={field.fieldName} defaultValue={defaultValue} placeholder={field.placeholder} onChange={this.onChange.bind(this, field.fieldName)} />
                );
         }
     }
 
     render() {
-        let { engine, hiddenFields, submitButtonText } = this.props;
+        let { details, engine, formError, hiddenFields, submitButtonText } = this.props;
+        let { valid } = this.state;
+
         hiddenFields = hiddenFields || [];
+        let existingName = (details && 'name' in details) ? details.name : "";
 
         return (
             <form onSubmit={this.formSubmitted.bind(this)} noValidate>
                 <FormField fieldName="name">
                     <FormLabel title="Name" fieldName="name"></FormLabel>
-                    <input className="Form-input Form-offset full" ref="name" name="name" placeholder="How would you like to refer to this database?" required autofocus />
+                    <input className="Form-input Form-offset full" ref="name" name="name" defaultValue={existingName} placeholder="How would you like to refer to this database?" required autofocus />
                     <span className="Form-charm"></span>
                 </FormField>
 
@@ -85,10 +147,10 @@ export default class DatabaseDetailsForm extends Component {
                 </div>
 
                 <div className="Form-actions">
-                    <button className="Button" mb-action-button="saveNoRedirect" success-text="Saved!" failed-text="Failed!" active-text="Validating " ng-disabled="!form.$valid || !database.engine">
+                    <button className={cx("Button", {"Button--primary": valid})} disabled={!valid}>
                         {submitButtonText}
                     </button>
-                    <FormMessage></FormMessage>
+                    <FormMessage formError={formError}></FormMessage>
                 </div>
             </form>
         );
@@ -96,8 +158,10 @@ export default class DatabaseDetailsForm extends Component {
 }
 
 DatabaseDetailsForm.propTypes = {
-    dispatch: PropTypes.func.isRequired,
+    details: PropTypes.object,
     engine: PropTypes.string.isRequired,
+    formError: PropTypes.object,
+    hiddenFields: PropTypes.array,
     submitButtonText: PropTypes.string.isRequired,
     submitFn: PropTypes.func.isRequired
 }

resources/frontend_client/app/setup/actions.js

@@ -35,7 +35,7 @@ function createThunkAction(actionType, actionThunkCreator) {
 }
 
 // // resource wrappers
-const SetupApi = new AngularResourceProxy("Setup", ["setup"]);
+const SetupApi = new AngularResourceProxy("Setup", ["create", "validate_db"]);
 
 
 // action constants
@@ -43,6 +43,7 @@ export const SET_ACTIVE_STEP = 'SET_ACTIVE_STEP';
 export const SET_USER_DETAILS = 'SET_USER_DETAILS';
 export const SET_DATABASE_DETAILS = 'SET_DATABASE_DETAILS';
 export const SET_ALLOW_TRACKING = 'SET_ALLOW_TRACKING';
+export const VALIDATE_DATABASE = 'VALIDATE_DATABASE';
 export const SUBMIT_SETUP = 'SUBMIT_SETUP';
 export const COMPLETE_SETUP = 'COMPLETE_SETUP';
 
@@ -53,12 +54,21 @@ export const setUserDetails = createAction(SET_USER_DETAILS);
 export const setDatabaseDetails = createAction(SET_DATABASE_DETAILS);
 export const setAllowTracking = createAction(SET_ALLOW_TRACKING);
 
+export const validateDatabase = createThunkAction(VALIDATE_DATABASE, function(details) {
+    return async function(dispatch, getState) {
+        return await SetupApi.validate_db({
+            'token': MetabaseSettings.get('setup_token'),
+            'details': details
+        });
+    };
+});
+
 export const submitSetup = createThunkAction(SUBMIT_SETUP, function() {
     return async function(dispatch, getState) {
         let { allowTracking, databaseDetails, userDetails} = getState();
 
         try {
-            let response = await SetupApi.setup({
+            let response = await SetupApi.create({
                 'token': MetabaseSettings.get('setup_token'),
                 'prefs': {
                     'site_name': userDetails.site_name,

resources/frontend_client/app/setup/components/DatabaseStep.react.js

 "use strict";
 
 import React, { Component, PropTypes } from "react";
+import _ from "underscore";
 
 import DatabaseDetailsForm from "metabase/components/database/DatabaseDetailsForm.react";
 import FormField from "metabase/components/form/FormField.react";
@@ -8,7 +9,7 @@ import Icon from "metabase/components/Icon.react";
 import MetabaseCore from "metabase/lib/core";
 
 import CollapsedStep from "./CollapsedStep.react";
-import { setDatabaseDetails } from "../actions";
+import { setDatabaseDetails, validateDatabase } from "../actions";
 
 
 export default class DatabaseStep extends Component {
@@ -26,14 +27,35 @@ export default class DatabaseStep extends Component {
         });
     }
 
-    detailsCaptured(details) {
-        this.props.dispatch(setDatabaseDetails({
-            'nextStep': ++this.props.stepNumber,
-            'details': details
-        }));
+    async detailsCaptured(details) {
+        let databaseDetails = _.clone(details);
+        databaseDetails.engine = this.state.engine;
+
+        this.setState({
+            'formError': null
+        });
+
+        try {
+            // validate them first
+            await this.props.dispatch(validateDatabase(databaseDetails));
+
+            // now that they are good, store them
+            this.props.dispatch(setDatabaseDetails({
+                'nextStep': ++this.props.stepNumber,
+                'details': databaseDetails
+            }));
+        } catch (error) {
+            this.setState({
+                'formError': error
+            });
+        }
     }
 
     skipDatabase() {
+        this.setState({
+            'engine': ""
+        });
+
         this.props.dispatch(setDatabaseDetails({
             'nextStep': ++this.props.stepNumber,
             'details': null
@@ -59,7 +81,7 @@ export default class DatabaseStep extends Component {
 
     render() {
         let { activeStep, databaseDetails, dispatch, stepNumber } = this.props;
-        let { engine } = this.state;
+        let { engine, formError } = this.state;
 
         let stepText = 'Add your data';
         if (activeStep > stepNumber) {
@@ -88,7 +110,14 @@ export default class DatabaseStep extends Component {
                         </FormField>
 
                         { engine !== "" ?
-                            <DatabaseDetailsForm engine={engine} hiddenFields={['ssl']} submitFn={this.detailsCaptured.bind(this)} submitButtonText={'Next'}></DatabaseDetailsForm>
+                            <DatabaseDetailsForm
+                                details={databaseDetails}
+                                engine={engine}
+                                formError={formError}
+                                hiddenFields={['ssl']}
+                                submitFn={this.detailsCaptured.bind(this)}
+                                submitButtonText={'Next'}>
+                            </DatabaseDetailsForm>
                         : null }
 
                         <div className="Form-field Form-offset">

resources/frontend_client/app/setup/setup.services.js

@@ -3,13 +3,13 @@
 var SetupServices = angular.module('metabase.setup.services', ['ngResource', 'ngCookies']);
 
 SetupServices.factory('Setup', ['$resource', '$cookies', function($resource, $cookies) {
-    return $resource('/api/setup/user', {}, {
-        create_user: {
+    return $resource('/api/setup/', {}, {
+        create: {
             method: 'POST'
         },
 
-        setup: {
-            url: '/api/setup/',
+        validate_db: {
+            url: '/api/setup/validate',
             method: 'POST'
         }
     });

src/metabase/api/setup.clj

@@ -2,11 +2,19 @@
   (:require [compojure.core :refer [defroutes POST]]
             [metabase.api.common :refer :all]
             [metabase.db :refer :all]
+            [metabase.driver :as driver]
             [metabase.events :as events]
-            (metabase.models [session :refer [Session]]
+            (metabase.models [database :refer [Database]]
+                             [session :refer [Session]]
+                             [setting :as setting]
                              [user :refer [User set-user-password]])
             [metabase.setup :as setup]
-            [metabase.util :as util]))
+            [metabase.util :as u]))
+
+(defannotation DBEngine
+  "Param must be a valid database engine type, e.g. `h2` or `postgres`."
+  [symb value :nillable]
+  (checkp-contains? (set (map name (keys driver/available-drivers))) symb value))
 
 (defannotation SetupToken
   "Check that param matches setup token or throw a 403."
@@ -17,12 +25,12 @@
 (defendpoint POST "/"
   "Special endpoint for creating the first user during setup.
    This endpoint both creates the user AND logs them in and returns a session ID."
-  [:as {{:keys [token] {:keys [first_name last_name email password]} :user :as body} :body, :as request}]
-  {first_name [Required NonEmptyString]
+  [:as {{:keys [token] {:keys [name engine details]} :database {:keys [first_name last_name email password]} :user {:keys [allow_tracking site_name]} :prefs} :body, :as request}]
+  {token      [Required SetupToken]
+   first_name [Required NonEmptyString]
    last_name  [Required NonEmptyString]
    email      [Required Email]
-   password   [Required ComplexPassword]
-   token      [Required SetupToken]}
+   password   [Required ComplexPassword]}
   ;; Call (metabase.core/site-url request) to set the Site URL setting if it's not already set
   (@(ns-resolve 'metabase.core 'site-url) request)
   ;; Now create the user
@@ -35,6 +43,13 @@
                      :is_superuser true)]
     ;; this results in a second db call, but it avoids redundant password code so figure it's worth it
     (set-user-password (:id new-user) password)
+    ;; set a couple preferences
+    (setting/set :site-name site_name)
+    (setting/set :anon-tracking-enabled allow_tracking)
+    ;; setup database (if needed)
+    (when (driver/is-engine? engine)
+      (->> (ins Database :name name :engine engine :details details)
+           (events/publish-event :database-create)))
     ;; clear the setup token now, it's no longer needed
     (setup/token-clear)
     ;; then we create a session right away because we want our new user logged in to continue the setup process
@@ -47,4 +62,24 @@
     {:id session-id}))
 
 
+(defendpoint POST "/validate"
+  "Validate that we can connect to a database given a set of details."
+  [:as {{{:keys [host port engine] :as details} :details token :token} :body}]
+  {token      [Required SetupToken]
+   engine     [Required DBEngine]}
+  (let [engine           (keyword engine)
+        details          (assoc details :engine engine)
+        response-invalid (fn [field m] {:status 400 :body (if (= :general field)
+                                                            {:message m}
+                                                            {:errors {field m}})})]
+    (try
+      (cond
+        (driver/can-connect-with-details? engine details :rethrow-exceptions) {:valid true}
+        (and host port (u/host-port-up? host port))                           (response-invalid :dbname  (format "Connection to '%s:%d' successful, but could not connect to DB." host port))
+        (and host (u/host-up? host))                                          (response-invalid :port    (format "Connection to '%s' successful, but port %d is invalid." port))
+        host                                                                  (response-invalid :host    (format "'%s' is not reachable" host))
+        :else                                                                 (response-invalid :general "Unable to connect to database."))
+      (catch Throwable e
+        (response-invalid :general (.getMessage e))))))
+
 (define-routes)

src/metabase/driver.clj

@@ -30,6 +30,10 @@
    :mysql    {:id   "mysql"
               :name "MySQL"}})
 
+(defn is-engine? [engine]
+  "Predicate function which validates if the given argument represents a valid driver identifier."
+  (contains? (set (map name (keys available-drivers))) (name engine)))
+
 (defn class->base-type
   "Return the `Field.base_type` that corresponds to a given class returned by the DB."
   [klass]

test/metabase/api/setup_test.clj

@@ -19,54 +19,63 @@
                (:id)
                (sel :one Session :user_id))
       {:id $id})
-    (http/client :post 200 "setup/user" {:token setup-token
-                                         :first_name user-name
-                                         :last_name user-name
-                                         :email (str user-name "@metabase.com")
-                                         :password "anythingUP12!!"})))
+    (http/client :post 200 "setup" {:token setup-token
+                                    :first_name user-name
+                                    :last_name user-name
+                                    :email (str user-name "@metabase.com")
+                                    :password "anythingUP12!!"})))
 
 
 ;; Test input validations
+(expect {:errors {:token "field is a required param."}}
+  (http/client :post 400 "setup" {}))
+
+(expect {:errors {:token "Invalid value 'foobar' for 'token': Token does not match the setup token."}}
+  (http/client :post 400 "setup" {:token "foobar"}))
+
+;; all of these tests can reuse the same setup token
 (expect {:errors {:first_name "field is a required param."}}
-  (http/client :post 400 "setup/user" {}))
+  (http/client :post 400 "setup" {:token (setup/token-value)}))
 
 (expect {:errors {:last_name "field is a required param."}}
-  (http/client :post 400 "setup/user" {:first_name "anything"}))
+  (http/client :post 400 "setup" {:token (setup/token-value)
+                                  :user {:first_name "anything"}}))
 
 (expect {:errors {:email "field is a required param."}}
-  (http/client :post 400 "setup/user" {:first_name "anything"
-                                       :last_name "anything"}))
+  (http/client :post 400 "setup" {:token (setup/token-value)
+                                  :user {:first_name "anything"
+                                         :last_name "anything"}}))
 
 (expect {:errors {:password "field is a required param."}}
-  (http/client :post 400 "setup/user" {:first_name "anything"
-                                       :last_name "anything"
-                                       :email "anything@metabase.com"}))
-
-(expect {:errors {:token "field is a required param."}}
-  (http/client :post 400 "setup/user" {:first_name "anything"
-                                       :last_name "anything"
-                                       :email "anything@metabase.com"
-                                       :password "anythingUP12!!"}))
+  (http/client :post 400 "setup" {:token (setup/token-value)
+                                  :user {:first_name "anything"
+                                         :last_name "anything"
+                                         :email "anything@metabase.com"}}))
 
 ;; valid email + complex password
 (expect {:errors {:email "Invalid value 'anything' for 'email': Not a valid email address."}}
-  (http/client :post 400 "setup/user" {:token "anything"
-                                       :first_name "anything"
-                                       :last_name "anything"
-                                       :email "anything"
-                                       :password "anything"}))
+  (http/client :post 400 "setup" {:token (setup/token-value)
+                                  :user {:token "anything"
+                                         :first_name "anything"
+                                         :last_name "anything"
+                                         :email "anything"
+                                         :password "anything"}}))
 
 (expect {:errors {:password "Insufficient password strength"}}
-  (http/client :post 400 "setup/user" {:token "anything"
-                                       :first_name "anything"
-                                       :last_name "anything"
-                                       :email "anything@email.com"
-                                       :password "anything"}))
+  (http/client :post 400 "setup" {:token (setup/token-value)
+                                  :user {:token "anything"
+                                         :first_name "anything"
+                                         :last_name "anything"
+                                         :email "anything@email.com"
+                                         :password "anything"}}))
+
+
+;; ## POST /api/setup/validate
+(expect {:errors {:token "field is a required param."}}
+  (http/client :post 400 "setup/validate" {}))
+
+(expect {:errors {:token "Invalid value 'foobar' for 'token': Token does not match the setup token."}}
+  (http/client :post 400 "setup/validate" {:token "foobar"}))
 
-;; token match
-(expect {:errors {:token "Invalid value 'anything' for 'token': Token does not match the setup token."}}
-  (http/client :post 400 "setup/user" {:token "anything"
-                                       :first_name "anything"
-                                       :last_name "anything"
-                                       :email "anything@email.com"
-                                       :password "anythingUP12!!"}))
+(expect {:errors {:engine "field is a required param."}}
+  (http/client :post 400 "setup/validate" {:token (setup/token-value)}))