Use default values for applicable fields when creating DB

resources/frontend_client/app/metabase/metabase.services.js

@@ -3,7 +3,7 @@
 // Metabase Services
 var MetabaseServices = angular.module('corvus.metabase.services', ['ngResource', 'ngCookies']);
 
-MetabaseServices.factory('Metabase', ['$resource', '$cookies', function($resource, $cookies) {
+MetabaseServices.factory('Metabase', ['$resource', '$cookies', 'CorvusCore', function($resource, $cookies, CorvusCore) {
     return $resource('/api/meta', {}, {
         db_form_input: {
             url: '/api/meta/db/form_input',
@@ -21,6 +21,10 @@ MetabaseServices.factory('Metabase', ['$resource', '$cookies', function($resourc
                 'X-CSRFToken': function() {
                     return $cookies.csrftoken;
                 }
+            },
+            transformRequest: function(data) {
+                data = CorvusCore.prepareDatabaseDetails(data);
+                return angular.toJson(data);
             }
         },
         validate_connection: {
@@ -32,8 +36,7 @@ MetabaseServices.factory('Metabase', ['$resource', '$cookies', function($resourc
                 }
             },
             transformRequest: function(data) {
-                // API expects 'port' to be an int :imp:
-                if (data.port) data.port = parseInt(data.port);
+                data = CorvusCore.prepareDatabaseDetails(data);
                 return angular.toJson(data);
             }
         },

resources/frontend_client/app/services.js

@@ -533,11 +533,27 @@ CorvusServices.service('CorvusCore', ['$resource', 'User', function($resource, U
     // Until this is fully supported by the backend(s), we can save the connection details with both the 'new-style' broken-out values, and the combined conn_str
     // to ensure backwards-compatibility. Until this transition is complete, however, we'll still need to handle legacy maps containing just 'conn_str'.
     //
-    // each engine should define the following properties:
+    // ENGINE DICT FORMAT:
     // *  name         - human-facing name to use for this DB engine
-    // *  fields       - list of available fields when a user adds/edits a DB of this type
     // *  buildDetails - take a 'new-style' details map and add 'conn_str' for backwards compatibility, if needed
     // *  parseDetails - take a details map and parse 'conn_str' if it's a legacy map. Otherwise we can return the map as-is
+    // *  fields       - array of available fields to display when a user adds/edits a DB of this type. Each field should be a dict of the format below:
+    //
+    // FIELD DICT FORMAT:
+    // *  displayName          - user-facing name for the Field
+    // *  fieldName            - name used for the field in a database details dict
+    // *  transform            - function to apply to this value before passing to the API, such as 'parseInt'. (default: none)
+    // *  placeholder          - placeholder value that should be used in text input for this field (default: none)
+    // *  placeholderIsDefault - if true, use the value of 'placeholder' as the default value of this field if none is specified (default: false)
+    //                           (if you set this, don't set 'required', or user will still have to add a value for the field)
+    // *  required             - require the user to enter a value for this field? (default: false)
+    // *  choices              - array of possible values for this field. If provided, display a button toggle instead of a text input.
+    //                           Each choice should be a dict of the format below: (optional)
+    //
+    // CHOICE DICT FORMAT:
+    // *  name            - User-facing name for the choice.
+    // *  value           - Value to use for the choice in the database connection details dict.
+    // *  selectionAccent - What accent type should be applied to the field when its value is chosen? Either 'active' (currently green), or 'danger' (currently red).
     this.ENGINES = {
         postgres: {
             name: 'Postgres',
@@ -545,12 +561,13 @@ CorvusServices.service('CorvusCore', ['$resource', 'User', function($resource, U
                 displayName: "Host",
                 fieldName: "host",
                 placeholder: "localhost",
-                required: true
+                placeholderIsDefault: true
             }, {
                 displayName: "Port",
                 fieldName: "port",
+                transform: parseInt,
                 placeholder: "5432",
-                required: true
+                placeholderIsDefault: true
             }, {
                 displayName: "Database name",
                 fieldName: "dbname",
@@ -596,7 +613,12 @@ CorvusServices.service('CorvusCore', ['$resource', 'User', function($resource, U
             },
             buildDetails: function(details) {
                 // add conn_str for backwards-compatibility
-                details.conn_str = "host=" + details.host + " port=" + details.port + " dbname=" + details.dbname + " user=" + details.user + (details.pass ? (" password=" + details.pass) : '');
+                details.conn_str =
+                    "host=" + details.host +
+                    " port=" + details.port +
+                    " dbname=" + details.dbname +
+                    " user=" + details.user +
+                    (details.pass ? (" password=" + details.pass) : '');
                 return details;
             }
         },
@@ -628,10 +650,11 @@ CorvusServices.service('CorvusCore', ['$resource', 'User', function($resource, U
                 displayName: "Host",
                 fieldName: "host",
                 placeholder: "localhost",
-                required: true
+                placeholderIsDefault: true
             }, {
                 displayName: "Port",
                 fieldName: "port",
+                transform: parseInt,
                 placeholder: "27017"
             }, {
                 displayName: "Database name",
@@ -683,6 +706,31 @@ CorvusServices.service('CorvusCore', ['$resource', 'User', function($resource, U
             }
         }
     };
+
+    // Prepare database details before being sent to the API.
+    // This includes applying 'transform' functions and adding default values where applicable.
+    this.prepareDatabaseDetails = function(details) {
+        if (!details.engine) throw "Missing key 'engine' in database request details; please add this as API expects it in the request body.";
+
+        // iterate over each field definition
+        this.ENGINES[details.engine].fields.forEach(function(field) {
+            var fieldName = field.fieldName;
+
+            // set default value if applicable
+            if (!details[fieldName] && field.placeholderIsDefault) {
+                console.log('Setting default ', fieldName, ' -> ', field.placeholder);
+                details[fieldName] = field.placeholder;
+            }
+
+            // apply transformation function if applicable
+            if (details[fieldName] && field.transform) {
+                console.log('Applying transform to ', fieldName, ' -> ', field.transform(details[fieldName]));
+                details[fieldName] = field.transform(details[fieldName]);
+            }
+        });
+
+        return details;
+    };
 }]);
 
 CorvusServices.service('CorvusAlert', [function() {

resources/frontend_client/app/setup/setup.controllers.js

@@ -161,7 +161,7 @@ SetupControllers.controller('SetupConnection', ['$scope', '$routeParams', '$loca
 
         function displayConnectionValidationError(error) {
             console.log(error);
-            $scope.error = "Invalid Connection String - " + error.data.message;
+            $scope.error = 'Connection failed: ' + error.data.message;
         }
 
         // now perform the connection validation

src/metabase/api/meta/db.clj

@@ -56,18 +56,18 @@
 (defendpoint POST "/validate"
   "Validate that we can connect to a `Database`."
   [:as {{:keys [host port engine] :as details} :body}]
-  {host Required
-   port Required}
-  (let [details (assoc details :engine (keyword engine))
+  (let [engine           (keyword engine)
+        details          (assoc details :engine engine)
         response-invalid (fn [field m] {:status 400 :body {:valid false
                                                           field m        ; work with the new {:field error-message} format
                                                           :message m}})] ; but be backwards-compatible with the UI as it exists right now
     (try
       (cond
-        (driver/can-connect-with-details? (keyword engine) details) {:valid true}
-        (u/host-port-up? host port)                                 (response-invalid :dbname "Invalid connection details")
-        (u/host-up? host)                                           (response-invalid :port "Invalid port")
-        :else                                                       (response-invalid :host "Host not reachable"))
+        (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 :db     "Unable to connect to database."))
       (catch Throwable e
         (response-invalid :dbname (.getMessage e))))))
 

src/metabase/driver.clj

@@ -94,9 +94,11 @@
 
 (defn can-connect-with-details?
   "Check whether we can connect to a database with ENGINE and DETAILS-MAP and perform a basic query.
+   Specify optional param RETHROW-EXCEPTIONS if you want to handle any exceptions thrown yourself
+   (e.g., so you can pass the exception message along to the user).
 
      (can-connect-with-details? :postgres {:host \"localhost\", :port 5432, ...})"
-  [engine details-map]
+  [engine details-map & [rethrow-exceptions]]
   {:pre [(keyword? engine)
          (contains? (set (keys available-drivers)) engine)
          (map? details-map)]}
@@ -104,6 +106,8 @@
     (i/can-connect-with-details? (engine->driver engine) details-map)
     (catch Throwable e
       (log/error "Failed to connect to database:" (.getMessage e))
+      (when rethrow-exceptions
+        (throw e))
       false)))
 
 (def ^{:arglists '([database])} sync-database!