stubbing out segment api.

src/metabase/api/routes.clj

@@ -12,6 +12,7 @@
                           [notify :as notify]
                           [pulse :as pulse]
                           [revision :as revision]
+                          [segment :as segment]
                           [session :as session]
                           [setting :as setting]
                           [setup :as setup]
@@ -45,6 +46,7 @@
   (context "/notify"       [] (+apikey notify/routes))
   (context "/pulse"        [] (+auth pulse/routes))
   (context "/revision"     [] (+auth revision/routes))
+  (context "/segment"      [] (+auth segment/routes))
   (context "/session"      [] session/routes)
   (context "/setting"      [] (+auth setting/routes))
   (context "/setup"        [] setup/routes)

src/metabase/api/segment.clj

+(ns metabase.api.segment
+  "/api/segment endpoints."
+  (:require [compojure.core :refer [defroutes GET PUT POST DELETE]]
+            [metabase.api.common :refer :all]
+            [metabase.db :as db]
+            (metabase.models [segment :refer [Segment] :as segment]
+                             [table :refer [Table]])))
+
+
+;(defendpoint GET "/"
+;  "Fetch all `Segments`"
+;  []
+;  (check-superuser)
+;  (segment/retrieve-segments))
+
+
+(defendpoint POST "/"
+  "Create a new `Segment`."
+  [:as {{:keys [name description table_id definition] :as body} :body}]
+  {name       [Required NonEmptyString]
+   table_id   [Required Integer]
+   definition [Required Dict]}
+  (check-superuser)
+  (checkp #(db/exists? Table :id table_id) "table_id" "Table does not exist.")
+  (->500 (segment/create-segment table_id name description *current-user-id* definition)))
+
+
+;(defendpoint GET "/:id"
+;  "Fetch `Segment` with ID."
+;  [id]
+;  (check-superuser)
+;  (->404 (segment/retrieve-segment id)))
+
+
+(defendpoint PUT "/:id"
+  "Update a `Segment` with ID."
+  [id :as {{:keys [name description definition revision_message] :as body} :body}]
+  {name             [Required NonEmptyString]
+   revision_message [Required NonEmptyString]
+   definition       [Required Dict]}
+  (check-superuser)
+  (check-404 (db/exists? Segment :id id))
+  (segment/update-segment {:id          id
+                           :name        name
+                           :description description
+                           :definition  definition} revision_message))
+
+
+(defendpoint DELETE "/:id"
+  "Delete a `Segment`."
+  [id]
+  (check-superuser)
+  (let-404 [segment (db/sel :one Segment :id id)]
+    (segment/delete-segment id)))
+
+
+(define-routes)

src/metabase/models/segment.clj

@@ -45,6 +45,23 @@
 
 ;; ## Persistence Functions
 
+(defn create-segment
+  "Create a new `Segment`.
+
+   Returns the newly created `Segment` or throws an Exception."
+  [table-id name description creator-id definition]
+  {:pre [(integer? table-id)
+         (string? name)
+         (integer? creator-id)
+         (map? definition)]}
+  (-> (db/ins Segment
+        :table_id    table-id
+        :creator_id  creator-id
+        :name        name
+        :description description
+        :definition  definition)
+      (hydrate :creator)))
+
 (defn retrieve-segment
   "Fetch a single `Segment` by its ID value."
   [id]
@@ -78,19 +95,20 @@
     (->> (retrieve-segment id)
          (events/publish-event :segment-update))))
 
-(defn create-segment
-  "Create a new `Segment`.
+(defn delete-segment
+  "Delete a `Segment`.
 
-   Returns the newly created `Segment` or throws an Exception."
-  [table-id name description creator-id definition]
-  {:pre [(integer? table-id)
-         (string? name)
-         (integer? creator-id)
-         (map? definition)]}
-  (-> (db/ins Segment
-        :table_id    table-id
-        :creator_id  creator-id
-        :name        name
-        :description description
-        :definition  definition)
-      (hydrate :creator)))
+   This does a soft delete and simply marks the `Segment` as deleted but does not actually remove the
+   record from the database at any time.
+
+   Returns the final state of the `Segment` is successful, or throws an Exception."
+  [id]
+  {:pre [(integer? id)]}
+  (kdb/transaction
+    ;; make Segment not active
+    (db/upd Segment id
+      :active false)
+    ;; TODO: create a new revision
+    ;; fetch the fully updated segment and return it (and fire off an event)
+    (->> (retrieve-segment id)
+         (events/publish-event :segment-delete))))

test/metabase/api/segment_test.clj

+(ns metabase.api.segment-test
+  "Tests for /api/segment endpoints."
+  (:require [clojure.tools.macro :refer [symbol-macrolet]]
+            [expectations :refer :all]
+            (metabase [http-client :as http]
+                      [middleware :as middleware])
+            (metabase.models [database :refer [Database]]
+                             [segment :refer [Segment]]
+                             [table :refer [Table]])
+            [metabase.test.util :as tu]
+            [metabase.test.data.users :refer :all]
+            [metabase.test.data :refer :all]))
+
+;; ## Helper Fns
+
+(defn user-details [user]
+  (tu/match-$ user
+    {:id $
+     :email $
+     :date_joined $
+     :first_name $
+     :last_name $
+     :last_login $
+     :is_superuser $
+     :common_name $}))
+
+(defn segment-response [{:keys [created_at updated_at] :as segment}]
+  (-> segment
+      (dissoc :id :table_id)
+      (assoc :created_at (not (nil? created_at)))
+      (assoc :updated_at (not (nil? updated_at)))))
+
+
+;; ## /api/segment/* AUTHENTICATION Tests
+;; We assume that all endpoints for a given context are enforced by the same middleware, so we don't run the same
+;; authentication test on every single individual endpoint
+
+(expect (get middleware/response-unauthentic :body) (http/client :get 401 "segment"))
+(expect (get middleware/response-unauthentic :body) (http/client :put 401 "segment/13"))
+
+
+;; ## POST /api/segment
+
+;; test security.  requires superuser perms
+(expect "You don't have permissions to do that."
+  ((user->client :rasta) :post 403 "segment" {:name       "abc"
+                                              :table_id   123
+                                              :definition {}}))
+
+;; test validations
+(expect {:errors {:name "field is a required param."}}
+  ((user->client :crowberto) :post 400 "segment" {}))
+
+(expect {:errors {:table_id "field is a required param."}}
+  ((user->client :crowberto) :post 400 "segment" {:name "abc"}))
+
+(expect {:errors {:table_id "Invalid value 'foobar' for 'table_id': value must be an integer."}}
+  ((user->client :crowberto) :post 400 "segment" {:name     "abc"
+                                                  :table_id "foobar"}))
+
+(expect {:errors {:definition "field is a required param."}}
+  ((user->client :crowberto) :post 400 "segment" {:name     "abc"
+                                                  :table_id 123}))
+
+(expect {:errors {:definition "Invalid value 'foobar' for 'definition': value must be a dictionary."}}
+  ((user->client :crowberto) :post 400 "segment" {:name       "abc"
+                                                  :table_id   123
+                                                  :definition "foobar"}))
+
+(expect
+  {:name         "A Segment"
+   :description  "I did it!"
+   :creator_id   (user->id :crowberto)
+   :creator      (user-details (fetch-user :crowberto))
+   :created_at   true
+   :updated_at   true
+   :definition   {:database 21
+                  :query    {:filter ["abc"]}}}
+  (tu/with-temp Database [{database-id :id} {:name      "Hillbilly"
+                                             :engine    :yeehaw
+                                             :details   {}
+                                             :is_sample false}]
+    (tu/with-temp Table [{:keys [id]} {:name   "Stuff"
+                                       :db_id  database-id
+                                       :active true}]
+      (segment-response ((user->client :crowberto) :post 200 "segment" {:name        "A Segment"
+                                                                        :description "I did it!"
+                                                                        :table_id    id
+                                                                        :definition  {:database 21
+                                                                                      :query    {:filter ["abc"]}}})))))
+
+
+;; ## PUT /api/segment
+
+;; test security.  requires superuser perms
+(expect "You don't have permissions to do that."
+  ((user->client :rasta) :put 403 "segment/1" {:name             "abc"
+                                               :definition       {}
+                                               :revision_message "something different"}))
+
+;; test validations
+(expect {:errors {:name "field is a required param."}}
+  ((user->client :crowberto) :put 400 "segment/1" {}))
+
+(expect {:errors {:revision_message "field is a required param."}}
+  ((user->client :crowberto) :put 400 "segment/1" {:name "abc"}))
+
+(expect {:errors {:revision_message "Invalid value '' for 'revision_message': value must be a non-empty string."}}
+  ((user->client :crowberto) :put 400 "segment/1" {:name             "abc"
+                                                   :revision_message ""}))
+
+(expect {:errors {:definition "field is a required param."}}
+  ((user->client :crowberto) :put 400 "segment/1" {:name             "abc"
+                                                   :revision_message "123"}))
+
+(expect {:errors {:definition "Invalid value 'foobar' for 'definition': value must be a dictionary."}}
+  ((user->client :crowberto) :put 400 "segment/1" {:name             "abc"
+                                                   :revision_message "123"
+                                                   :definition       "foobar"}))
+
+(expect
+  {:name         "Tatooine"
+   :description  nil
+   :creator_id   (user->id :rasta)
+   :creator      (user-details (fetch-user :rasta))
+   :created_at   true
+   :updated_at   true
+   :definition   {:database 2
+                  :query    {:filter ["not" "the droids you're looking for"]}}}
+  (tu/with-temp Database [{database-id :id} {:name      "Hillbilly"
+                                             :engine    :yeehaw
+                                             :details   {}
+                                             :is_sample false}]
+    (tu/with-temp Table [{table-id :id} {:name   "Stuff"
+                                         :db_id  database-id
+                                         :active true}]
+      (tu/with-temp Segment [{:keys [id]} {:creator_id  (user->id :rasta)
+                                           :table_id    table-id
+                                           :name        "Droids in the desert"
+                                           :description "Lookin' for a jedi"
+                                           :definition  {}}]
+        (segment-response ((user->client :crowberto) :put 200 (format "segment/%d" id) {:id               id
+                                                                                        :name             "Tatooine"
+                                                                                        :description      nil
+                                                                                        :table_id         456
+                                                                                        :revision_message "I got me some revisions"
+                                                                                        :definition       {:database 2
+                                                                                                           :query    {:filter ["not" "the droids you're looking for"]}}}))))))
+
+
+;; ## DELETE /api/segment/:id
+
+
+;; ## GET /api/segment
+
+
+;; ## GET /api/segment/:id
+