Merge branch 'master' into release-0.25.0

bin/ci

@@ -155,6 +155,9 @@ trap summary EXIT
 fail_fast() {
   echo -e "========================================"
   echo -e "Failing fast! Stopping other nodes..."
+  # Touch a file to differentiate between a local failure and a
+  # failure triggered by another node
+  touch '/tmp/local-fail'
   # ssh to the other CircleCI nodes and send SIGUSR1 to tell them to exit early
   for (( i = 0; i < $CIRCLE_NODE_TOTAL; i++ )); do
     if [ $i != $CIRCLE_NODE_INDEX ]; then
@@ -182,7 +185,12 @@ fi
 
 export CIRCLE_COMMIT_MESSAGE="$(git log --format=oneline -n 1 $CIRCLE_SHA1)"
 
-if [ -f "/tmp/fail" ]; then
+# This local-fail check is to guard against two nodes failing at the
+# same time. Both nodes ssh to each node and drop /tmp/fail. Those
+# failing nodes then get here and see and the other node has told it
+# to exit early. This results in both nodes exiting early, and thus
+# not failing, causing the build to succeed
+if [[ -f "/tmp/fail" && ! -f "/tmp/local-fail" ]]; then
   exit_early
 fi
 

docs/developers-guide-osx.md

@@ -6,6 +6,8 @@ NOTE: These instructions are only for packaging a built Metabase uberjar into `M
 
 1.  Install XCode.
 
+1.  Install XCode command-line tools. In `Xcode` > `Preferences` > `Locations` select your current Xcode version in the `Command Line Tools` drop-down.
+
 1.  Run `./bin/build` to build the latest version of the uberjar.
 
 1.  Next, you'll need to run the following commands before building the app:
@@ -45,10 +47,22 @@ aws configure --profile metabase
 cp /path/to/private/key.pem OSX/dsa_priv.pem
 ```
 
-You'll probably also want an Apple Developer ID Application Certificate in your computer's keychain. You'll need to generate a Certificate Signing Request from Keychain Access, and have Sameer go to [the Apple Developer Site](https://developer.apple.com/account/mac/certificate/) and generate one for you, then load the file on your computer.
+You'll need the `Apple Developer ID Application Certificate` in your computer's keychain.
+You'll need to generate a Certificate Signing Request from Keychain Access, and have Sameer go to [the Apple Developer Site](https://developer.apple.com/account/mac/certificate/)
+and generate one for you, then load the file on your computer.
+
+Finally, you may need to open the project a single time in Xcode to make sure the appropriate "build schemes" are generated (these are not checked into CI).
+Run `open OSX/Metabase.xcodeproj` to open the project, which will automatically generate the appropriate schemes. This only needs to be done once.
 
 After that, you are good to go:
 ```bash
 # Bundle entire app, and upload to s3
 ./bin/osx-release
 ```
+
+## Debugging ./bin/osx-release
+
+*  You can run individual steps of the release script by passing in the appropriate step subroutines. e.g. `./bin/osx-release create_dmg upload`.
+   The entire sequence of different steps can be found at the bottom of `./bin/osx-release`.
+*  Generating the DMG seems to be somewhat finicky, so if it fails with a message like "Device busy" trying the step again a few times usually resolves the issue.
+   You can continue the build process from the DMG creation step by running `./bin/osx-release create_dmg upload`.

docs/users-guide/04-asking-questions.md

@@ -17,7 +17,7 @@ The first dropdown menu in the question builder is where you’ll choose the dat
 
 If you've [saved some questions](06-sharing-answers.html), in the Data menu you'll see the option to use one of your saved questions as source data. What this means in practice is that you can do things like use complex SQL queries to create new tables that can be used in a question just like any other table in your database.
 
-You can use any saved question as source data, provided you have [permission](../administration-guide/05-setting-permissions.html) to view that question. You can even use questions that were saved as a chart rather than a table. The only caveat is that you can't use a saved question which itself uses a saved question as source data. (That's more inception than Metabase can handle!)
+You can use any saved question as source data, provided you have [permission](../administration-guide/05-setting-permissions.html) to view that question. You can even use questions that were saved as a chart rather than a table.
 
 ### Filters
 ---

frontend/src/metabase/redux/metadata.js

@@ -146,6 +146,27 @@ export const fetchDatabases = createThunkAction(FETCH_DATABASES, (reload = false
     };
 });
 
+export const FETCH_REAL_DATABASES = "metabase/metadata/FETCH_REAL_DATABASES";
+export const fetchRealDatabases = createThunkAction(FETCH_REAL_DATABASES, (reload = false) => {
+    return async (dispatch, getState) => {
+        const requestStatePath = ["metadata", "databases"];
+        const existingStatePath = requestStatePath;
+        const getData = async () => {
+            const databases = await MetabaseApi.db_real_list_with_tables();
+            return normalize(databases, [DatabaseSchema]);
+        };
+
+        return await fetchData({
+            dispatch,
+            getState,
+            requestStatePath,
+            existingStatePath,
+            getData,
+            reload
+        });
+    };
+});
+
 export const FETCH_DATABASE_METADATA = "metabase/metadata/FETCH_DATABASE_METADATA";
 export const fetchDatabaseMetadata = createThunkAction(FETCH_DATABASE_METADATA, function(dbId, reload = false) {
     return async function(dispatch, getState) {

frontend/src/metabase/reference/databases/databases.integ.spec.js

@@ -10,7 +10,7 @@ import { CardApi } from 'metabase/services'
 
 import { 
     FETCH_DATABASE_METADATA,
-    FETCH_DATABASES
+    FETCH_REAL_DATABASES
 } from "metabase/redux/metadata";
 
 import { END_LOADING } from "metabase/reference/reference"
@@ -48,7 +48,7 @@ describe("The Reference Section", () => {
             const store = await createTestStore()
             store.pushPath("/reference/databases/");
             var container = mount(store.connectContainer(<DatabaseListContainer />));
-            await store.waitForActions([FETCH_DATABASES, END_LOADING])
+            await store.waitForActions([FETCH_REAL_DATABASES, END_LOADING])
             
             expect(container.find(ReferenceHeader).length).toBe(1)
             expect(container.find(DatabaseList).length).toBe(1)            
@@ -58,6 +58,28 @@ describe("The Reference Section", () => {
             expect(container.find(ListItem).length).toBeGreaterThanOrEqual(1)
         })
         
+        // database list
+        it("should not see saved questions in the database list", async () => {
+            var card = await CardApi.create(cardDef)
+            const store = await createTestStore()
+            store.pushPath("/reference/databases/");
+            var container = mount(store.connectContainer(<DatabaseListContainer />));
+            await store.waitForActions([FETCH_REAL_DATABASES, END_LOADING])
+            
+            expect(container.find(ReferenceHeader).length).toBe(1)
+            expect(container.find(DatabaseList).length).toBe(1)            
+            expect(container.find(AdminAwareEmptyState).length).toBe(0)
+            
+            expect(container.find(List).length).toBe(1)
+            expect(container.find(ListItem).length).toBe(1)
+
+
+            expect(card.name).toBe(cardDef.name);
+            
+            await CardApi.delete({cardId: card.id})
+
+        })
+        
         // database detail
         it("should see a the detail view for the sample database", async ()=>{
             const store = await createTestStore()

frontend/src/metabase/reference/reference.js

@@ -176,7 +176,7 @@ export const wrappedFetchMetricRevisions = async (props, metricID) => {
 // }
 
 export const wrappedFetchDatabases = (props) => {
-    fetchDataWrapper(props, props.fetchDatabases)({})
+    fetchDataWrapper(props, props.fetchRealDatabases)({})
 }
 export const wrappedFetchMetrics = (props) => {
     fetchDataWrapper(props, props.fetchMetrics)({})

frontend/src/metabase/services.js

@@ -95,6 +95,7 @@ export const LdapApi = {
 export const MetabaseApi = {
     db_list:                     GET("/api/database"),
     db_list_with_tables:         GET("/api/database?include_tables=true&include_cards=true"),
+    db_real_list_with_tables:    GET("/api/database?include_tables=true&include_cards=false"),
     db_create:                  POST("/api/database"),
     db_add_sample_dataset:      POST("/api/database/sample_dataset"),
     db_get:                      GET("/api/database/:dbId"),

frontend/src/metabase/visualizations/visualizations/PieChart.jsx

@@ -46,7 +46,7 @@ export default class PieChart extends Component {
 
     static checkRenderable([{ data: { cols, rows} }], settings) {
         if (!settings["pie.dimension"] || !settings["pie.metric"]) {
-            throw new ChartSettingsError("Which columns do want to use?", "Data");
+            throw new ChartSettingsError("Which columns do you want to use?", "Data");
         }
     }
 

src/metabase/api/field.clj

@@ -126,7 +126,7 @@
                         human_readable_field_id))
       [400 "Foreign key based remappings require a human readable field id"])
     (if-let [dimension (Dimension :field_id id)]
-      (db/update! Dimension (:id dimension)
+      (db/update! Dimension (u/get-id dimension)
         {:type dimension-type
          :name dimension-name
          :human_readable_field_id human_readable_field_id})
@@ -174,10 +174,10 @@
                                               (map second value-pairs))))))
 
 (defn- create-field-values!
-  [field value-pairs]
+  [field-or-id value-pairs]
   (let [human-readable-values? (validate-human-readable-pairs value-pairs)]
     (db/insert! FieldValues
-      :field_id (:id field)
+      :field_id (u/get-id field-or-id)
       :values (map first value-pairs)
       :human_readable_values (when human-readable-values?
                                (map second value-pairs)))))

src/metabase/api/session.clj

@@ -40,6 +40,28 @@
   {:username   (throttle/make-throttler :username)
    :ip-address (throttle/make-throttler :username, :attempts-threshold 50)}) ; IP Address doesn't have an actual UI field so just show error by username
 
+(defn- ldap-login
+  "If LDAP is enabled and a matching user exists return a new Session for them, or `nil` if they couldn't be authenticated."
+  [username password]
+  (when (ldap/ldap-configured?)
+    (try
+      (when-let [user-info (ldap/find-user username)]
+        (when-not (ldap/verify-password user-info password)
+          ;; Since LDAP knows about the user, fail here to prevent the local strategy to be tried with a possibly outdated password
+          (throw (ex-info "Password did not match stored password." {:status-code 400
+                                                                     :errors      {:password "did not match stored password"}})))
+        ;; password is ok, return new session
+        {:id (create-session! (ldap/fetch-or-create-user! user-info password))})
+      (catch com.unboundid.util.LDAPSDKException e
+        (log/error (u/format-color 'red "Problem connecting to LDAP server, will fallback to local authentication") (.getMessage e))))))
+
+(defn- email-login
+  "Find a matching `User` if one exists and return a new Session for them, or `nil` if they couldn't be authenticated."
+  [username password]
+  (when-let [user (db/select-one [User :id :password_salt :password :last_login], :email username, :is_active true)]
+    (when (pass/verify-password password (:password_salt user) (:password user))
+      {:id (create-session! user)})))
+
 (api/defendpoint POST "/"
   "Login."
   [:as {{:keys [username password]} :body, remote-address :remote-addr}]
@@ -48,28 +70,13 @@
   (throttle/check (login-throttlers :ip-address) remote-address)
   (throttle/check (login-throttlers :username)   username)
   ;; Primitive "strategy implementation", should be reworked for modular providers in #3210
-  (or
-    ;; First try LDAP if it's enabled
-    (when (ldap/ldap-configured?)
-      (try
-        (when-let [user-info (ldap/find-user username)]
-          (if (ldap/verify-password user-info password)
-            {:id (create-session! (ldap/fetch-or-create-user! user-info password))}
-            ;; Since LDAP knows about the user, fail here to prevent the local strategy to be tried with a possibly outdated password
-            (throw (ex-info "Password did not match stored password." {:status-code 400
-                                                                       :errors      {:password "did not match stored password"}}))))
-        (catch com.unboundid.util.LDAPSDKException e
-          (log/error (u/format-color 'red "Problem connecting to LDAP server, will fallback to local authentication") (.getMessage e)))))
-
-    ;; Then try local authentication
-    (when-let [user (db/select-one [User :id :password_salt :password :last_login], :email username, :is_active true)]
-      (when (pass/verify-password password (:password_salt user) (:password user))
-        {:id (create-session! user)}))
-
-    ;; If nothing succeeded complain about it
-    ;; Don't leak whether the account doesn't exist or the password was incorrect
-    (throw (ex-info "Password did not match stored password." {:status-code 400
-                                                               :errors      {:password "did not match stored password"}}))))
+  (or (ldap-login username password)  ; First try LDAP if it's enabled
+      (email-login username password) ; Then try local authentication
+      ;; If nothing succeeded complain about it
+      ;; Don't leak whether the account doesn't exist or the password was incorrect
+      (throw (ex-info "Password did not match stored password." {:status-code 400
+                                                                 :errors      {:password "did not match stored password"}}))))
+
 
 (api/defendpoint DELETE "/"
   "Logout."

src/metabase/api/table.clj

@@ -18,8 +18,7 @@
             [schema.core :as s]
             [toucan
              [db :as db]
-             [hydrate :refer [hydrate]]]
-            [metabase.query :as q]))
+             [hydrate :refer [hydrate]]]))
 
 ;; TODO - I don't think this is used for anything any more
 (def ^:private ^:deprecated TableEntityType

src/metabase/driver.clj

@@ -278,16 +278,19 @@
                  :features       (features driver)})
               @registered-drivers))
 
+(defn- init-driver-in-namespace! [ns-symb]
+  (require ns-symb)
+  (if-let [register-driver-fn (ns-resolve ns-symb (symbol "-init-driver"))]
+    (register-driver-fn)
+    (log/warn (format "No -init-driver function found for '%s'" (name ns-symb)))))
+
 (defn find-and-load-drivers!
   "Search Classpath for namespaces that start with `metabase.driver.`, then `require` them and look for the `driver-init`
    function which provides a uniform way for Driver initialization to be done."
   []
   (doseq [ns-symb @u/metabase-namespace-symbols
           :when   (re-matches #"^metabase\.driver\.[a-z0-9_]+$" (name ns-symb))]
-    (require ns-symb)
-    (if-let [register-driver-fn (ns-resolve ns-symb (symbol "-init-driver"))]
-      (register-driver-fn)
-      (log/warn (format "No -init-driver function found for '%s'" (name ns-symb))))))
+    (init-driver-in-namespace! ns-symb)))
 
 (defn is-engine?
   "Is ENGINE a valid driver name?"
@@ -351,8 +354,9 @@
   [engine]
   {:pre [engine]}
   (or ((keyword engine) @registered-drivers)
-      (let [namespce (symbol (format "metabase.driver.%s" (name engine)))]
-        (u/ignore-exceptions (require namespce))
+      (let [namespace-symb (symbol (format "metabase.driver.%s" (name engine)))]
+        ;; TODO - Maybe this should throw the Exception instead of swallowing it?
+        (u/ignore-exceptions (init-driver-in-namespace! namespace-symb))
         ((keyword engine) @registered-drivers))))
 
 

src/metabase/models/field.clj

@@ -7,14 +7,13 @@
              [util :as u]]
             [metabase.models
              [dimension :refer [Dimension]]
-             [field-values :refer [FieldValues] :as fv]
+             [field-values :as fv :refer [FieldValues]]
              [humanization :as humanization]
              [interface :as i]
              [permissions :as perms]]
             [toucan
              [db :as db]
-             [models :as models]]
-            [metabase.models.field-values :as fv]))
+             [models :as models]]))
 
 ;;; ------------------------------------------------------------ Type Mappings ------------------------------------------------------------
 

src/metabase/query_processor.clj

@@ -94,7 +94,7 @@
       add-dim/add-remapping
       implicit-clauses/add-implicit-clauses
       source-table/resolve-source-table-middleware
-      expand/expand-middleware                      ; ▲▲▲ QUERY EXPANSION POINT  ▲▲▲ All functions *above* will see EXPANDED query during PRE-PROCESSING
+      expand/expand-middleware                         ; ▲▲▲ QUERY EXPANSION POINT  ▲▲▲ All functions *above* will see EXPANDED query during PRE-PROCESSING
       row-count-and-status/add-row-count-and-status    ; ▼▼▼ RESULTS WRAPPING POINT ▼▼▼ All functions *below* will see results WRAPPED in `:data` during POST-PROCESSING
       parameters/substitute-parameters
       expand-macros/expand-macros

src/metabase/query_processor/interface.clj

@@ -3,8 +3,8 @@
    This namespace should just contain definitions of various protocols and record types; associated logic
    should go in `metabase.query-processor.middleware.expand`."
   (:require [metabase.models
-             [field :as field]
-             [dimension :as dim]]
+             [dimension :as dim]
+             [field :as field]]
             [metabase.util :as u]
             [metabase.util.schema :as su]
             [schema.core :as s])

src/metabase/query_processor/middleware/add_dimension_projections.clj

@@ -144,8 +144,4 @@
   query). Then delegates to `remap-results` to munge the results after
   query execution."
   [qp]
-  (fn [query]
-    (-> query
-        add-fk-remaps
-        qp
-        remap-results)))
+  (comp remap-results qp add-fk-remaps))