Typo fixes and other code cleanup [ci bigquery]

src/metabase/api/routes.clj

@@ -4,7 +4,7 @@
              [route :as route]]
             [metabase.api
              [activity :as activity]
-             [alert    :as alert]
+             [alert :as alert]
              [automagic-dashboards :as magic]
              [card :as card]
              [collection :as collection]
@@ -30,8 +30,8 @@
              [setup :as setup]
              [slack :as slack]
              [table :as table]
-             [tiles :as tiles]
              [task :as task]
+             [tiles :as tiles]
              [user :as user]
              [util :as util]]
             [metabase.middleware :as middleware]

src/metabase/api/util.clj

@@ -29,7 +29,7 @@
   (stats/anonymous-usage-stats))
 
 (api/defendpoint GET "/random_token"
-  "Return a cryptographically secure random 32-byte token, encoded as a hexidecimal string.
+  "Return a cryptographically secure random 32-byte token, encoded as a hexadecimal string.
    Intended for use when creating a value for `embedding-secret-key`."
   []
   {:token (crypto-random/hex 32)})

src/metabase/email/messages.clj

@@ -14,15 +14,14 @@
             [metabase.util
              [date :as du]
              [export :as export]
-             [i18n :refer [tru]]
+             [i18n :refer [trs tru]]
              [quotation :as quotation]
              [urls :as url]]
             [stencil
              [core :as stencil]
              [loader :as stencil-loader]]
             [toucan.db :as db])
-  (:import [java.io File FileOutputStream IOException]
-           java.util.Arrays))
+  (:import [java.io File IOException]))
 
 ;; Dev only -- disable template caching
 (when config/is-dev?
@@ -36,19 +35,21 @@
     {:quotation       (:quote data-quote)
      :quotationAuthor (:author data-quote)}))
 
-(def ^:private ^:const notification-context
+(def ^:private notification-context
   {:emailType  "notification"
    :logoHeader true})
 
-(def ^:private ^:const abandonment-context
-  {:heading      "We’d love your feedback."
-   :callToAction "It looks like Metabase wasn’t quite a match for you. Would you mind taking a fast 5 question survey to help the Metabase team understand why and make things better in the future?"
-   :link         "http://www.metabase.com/feedback/inactive"})
+(defn- abandonment-context []
+  {:heading      (str (trs "We’d love your feedback."))
+   :callToAction (str (trs "It looks like Metabase wasn’t quite a match for you.")
+                      " "
+                      (trs "Would you mind taking a fast 5 question survey to help the Metabase team understand why and make things better in the future?"))
+   :link         "https://www.metabase.com/feedback/inactive"})
 
-(def ^:private ^:const follow-up-context
-  {:heading      "We hope you've been enjoying Metabase."
-   :callToAction "Would you mind taking a fast 6 question survey to tell us how it’s going?"
-   :link         "http://www.metabase.com/feedback/active"})
+(defn- follow-up-context []
+  {:heading      (str (trs "We hope you've been enjoying Metabase."))
+   :callToAction (str (trs "Would you mind taking a fast 6 question survey to tell us how it’s going?"))
+   :link         "https://www.metabase.com/feedback/active"})
 
 
 ;;; ### Public Interface
@@ -75,8 +76,9 @@
 
 (defn- all-admin-recipients
   "Return a sequence of email addresses for all Admin users.
-   The first recipient will be the site admin (or oldest admin if unset), which is the address that should be used in `mailto` links
-   (e.g., for the new user to email with any questions)."
+
+  The first recipient will be the site admin (or oldest admin if unset), which is the address that should be used in
+  `mailto` links (e.g., for the new user to email with any questions)."
   []
   (concat (when-let [admin-email (public-settings/admin-email)]
             [admin-email])
@@ -124,7 +126,8 @@
       :message-type :html
       :message      message-body)))
 
-;; TODO - I didn't write these function and I don't know what it's for / what it's supposed to be doing. If this is determined add appropriate documentation
+;; TODO - I didn't write these function and I don't know what it's for / what it's supposed to be doing. If this is
+;; determined add appropriate documentation
 
 (defn- model-name->url-fn [model]
   (case model
@@ -172,8 +175,8 @@
         context      (merge notification-context
                             (random-quote-context)
                             (if (= "abandon" msg-type)
-                              abandonment-context
-                              follow-up-context))
+                              (abandonment-context)
+                              (follow-up-context)))
         message-body (stencil/render-file "metabase/email/follow_up_email" context)]
     (email/send-message!
       :subject      subject

src/metabase/mbql/util.clj

@@ -265,7 +265,7 @@
     (throw
      (Exception.
       (str
-       (tru "Error: query's source query has not been resolved. You probably need to `preprocess` the query first."))))
+       (tru "Error: query''s source query has not been resolved. You probably need to `preprocess` the query first."))))
 
     ;; otherwise resolve the source Table
     :else

src/metabase/models/params.clj

@@ -5,6 +5,7 @@
              [db :as mdb]
              [util :as u]]
             [metabase.mbql.util :as mbql.u]
+            [metabase.util.i18n :as ui18n :refer [trs]]
             [toucan
              [db :as db]
              [hydrate :refer [hydrate]]]))
@@ -29,7 +30,7 @@
     field-form
 
     :else
-    (throw (IllegalArgumentException. (str "Don't know what to do with: " field-form)))))
+    (throw (IllegalArgumentException. (str (trs "Don't know what to do with:") " " field-form)))))
 
 (defn wrap-field-id-if-needed
   "Wrap a raw Field ID in a `:field-id` clause if needed."
@@ -42,7 +43,7 @@
     [:field-id field-id-or-form]
 
     :else
-    (throw (IllegalArgumentException. (str "Don't know how to wrap:" field-id-or-form)))))
+    (throw (IllegalArgumentException. (str (trs "Don't know how to wrap:") " " field-id-or-form)))))
 
 (defn- field-ids->param-field-values
   "Given a collection of PARAM-FIELD-IDS return a map of FieldValues for the Fields they reference.

src/metabase/models/permissions.clj

@@ -559,6 +559,7 @@
   [old-graph new-graph]
   (when (not= (:revision old-graph) (:revision new-graph))
     (throw (ui18n/ex-info (str (tru "Looks like someone else edited the permissions and your data is out of date.")
+                               " "
                                (tru "Please fetch new data and try again."))
              {:status-code 409}))))
 

src/metabase/models/user.clj

@@ -181,7 +181,7 @@
 
 (s/defn create-new-google-auth-user!
   "Convenience for creating a new user via Google Auth. This account is considered active immediately; thus all active
-  admins will recieve an email right away."
+  admins will receive an email right away."
   [new-user :- NewUser]
   (u/prog1 (insert-new-user! (assoc new-user :google_auth true))
     ;; send an email to everyone including the site admin if that's set
@@ -189,7 +189,7 @@
 
 (s/defn create-new-ldap-auth-user!
   "Convenience for creating a new user via LDAP. This account is considered active immediately; thus all active admins
-  will recieve an email right away."
+  will receive an email right away."
   [new-user :- NewUser]
   (insert-new-user! (-> new-user
                         ;; We should not store LDAP passwords

src/metabase/query_processor.clj

@@ -169,7 +169,7 @@
 
         ;; ...which ends up getting caught by the `catch-exceptions` middleware. Add a final post-processing function
         ;; around that which will return whatever we delivered into the `:results-promise`.
-        recieve-native-query
+        receive-native-query
         (fn [qp]
           (fn [query]
             (let [results-promise (promise)
@@ -183,7 +183,7 @@
                 ;; everyone a favor and throw an Exception
                 (let [results (m/dissoc-in results [:query :results-promise])]
                   (throw (ex-info (str (tru "Error preprocessing query")) results)))))))]
-    (recieve-native-query (qp-pipeline deliver-native-query))))
+    (receive-native-query (qp-pipeline deliver-native-query))))
 
 (defn query->preprocessed
   "Return the fully preprocessed form for `query`, the way it would look immediately before `mbql->native` is called.

src/metabase/query_processor/middleware/parameters/sql.clj

@@ -191,10 +191,10 @@
   "Return the `:default` value for a param if no explicit values were passsed. This only applies to non-Dimension
    (non-Field Filter) params. Default values for Dimension (Field Filter) params are handled above in
    `default-value-for-dimension`."
-  [{:keys [default display_name required]} :- TagParam]
+  [{:keys [default display-name required]} :- TagParam]
   (or default
       (when required
-        (throw (Exception. (str (tru "''{0}'' is a required param." display_name)))))))
+        (throw (Exception. (str (tru "''{0}'' is a required param." display-name)))))))
 
 
 ;;; Parsing Values

src/metabase/query_processor/util.clj

@@ -56,19 +56,6 @@
      (recur (assoc m :query source-query) ks not-found)
      (get-in m (cons :query ks) not-found))))
 
-(defn assoc-in-query
-  "Similar to `assoc-in but will look in either `:query` or recursively in `[:query :source-query]`. Using
-  this function will avoid having to check if there's a nested query vs. top-level query."
-  [m ks v]
-  (if-let [source-query (get-in m [:query :source-query])]
-    ;; We have a soure-query, we need to recursively `assoc-in` with the source query as the query
-    (assoc-in m
-              [:query :source-query]
-              (-> (assoc m :query source-query)
-                  (assoc-in-query ks v)
-                  :query))
-    (assoc-in m (cons :query ks) v)))
-
 
 ;;; ---------------------------------------------------- Hashing -----------------------------------------------------
 

test/metabase/driver/bigquery_test.clj

@@ -216,3 +216,28 @@
                                   :details (assoc (:details (Database (data/id)))
                                              :use-jvm-timezone true)}]]
       (native-timestamp-query db "2018-08-31 00:00:00+07" "Asia/Jakarta"))))
+
+;; if I run a BigQuery query, does it get a remark added to it?
+(expect-with-engine :bigquery
+  (str
+   "-- Metabase:: userID: 1000 queryType: MBQL queryHash: 01020304\n"
+   "SELECT `test_data.venues`.`id` AS `venues___id`,"
+   " `test_data.venues`.`name` AS `venues___name`,"
+   " `test_data.venues`.`category_id` AS `venues___category_id`,"
+   " `test_data.venues`.`latitude` AS `venues___latitude`,"
+   " `test_data.venues`.`longitude` AS `venues___longitude`,"
+   " `test_data.venues`.`price` AS `venues___price` "
+   "FROM `test_data.venues` "
+   "LIMIT 1")
+  (let [native-query (atom nil)]
+    (with-redefs [bigquery/process-native* (fn [_ sql]
+                                             (reset! native-query sql)
+                                             (throw (Exception. "Done.")))]
+      (qp/process-query {:database (data/id)
+                         :type     :query
+                         :query    {:source-table (data/id :venues)
+                                    :limit        1}
+                         :info     {:executed-by 1000
+                                    :query-type  "MBQL"
+                                    :query-hash  (byte-array [1 2 3 4])}})
+      @native-query)))

test/metabase/models/query/permissions_test.clj

@@ -13,9 +13,9 @@
             [metabase.models.query.permissions :as query-perms]
             [metabase.test.data :as data]
             [metabase.test.data.users :as users]
+            [metabase.test.util.log :as tu.log]
             [metabase.util :as u]
-            [toucan.util.test :as tt]
-            [metabase.test.util.log :as tu.log]))
+            [toucan.util.test :as tt]))
 
 ;;; ---------------------------------------------- Permissions Checking ----------------------------------------------
 

test/metabase/query_processor/util_test.clj

@@ -147,21 +147,3 @@
   (expect
     not-found
     (qputil/get-in-query {} [:test] not-found)))
-
-(def ^:private updated-test-map
-  {:test {:value 11}})
-
-;; assoc-in-query works with a non-nested query
-(expect
-  {:query updated-test-map}
-  (qputil/assoc-in-query {:query test-inner-map} [:test :value] 11))
-
-;; assoc-in-query works with a nested query
-(expect
-  {:query {:source-query updated-test-map}}
-  (qputil/assoc-in-query {:query {:source-query test-inner-map}} [:test :value] 11))
-
-;; Not supported yet, but assoc-in-query should do the right thing with a double nested query
-(expect
-  {:query {:source-query {:source-query updated-test-map}}}
-  (qputil/assoc-in-query {:query {:source-query {:source-query test-inner-map}}} [:test :value] 11))

test/metabase/test/data/dataset_definitions.clj

 (ns metabase.test.data.dataset-definitions
   "Definitions of various datasets for use in tests with `with-temp-db`."
-  (:require [clojure.tools.reader.edn :as edn]
-            [metabase.test.data.interface :as di]
-            [metabase.util.date :as du])
+  (:require [metabase.test.data.interface :as di])
   (:import java.sql.Time
            java.util.Calendar))
 
@@ -23,7 +21,7 @@
 (di/def-database-definition-edn places-cam-likes)
 
 ;; A small dataset with users and a set of messages between them. Each message has *2* foreign keys to user --
-;; sender and reciever -- allowing us to test situations where multiple joins for a *single* table should occur.
+;; sender and receiver -- allowing us to test situations where multiple joins for a *single* table should occur.
 (di/def-database-definition-edn avian-singles)
 
 (defn- date-only