add in support for soft-delete of a segment which simply sets :active false

resources/migrations/021_add_segment_table.yaml

@@ -41,6 +41,12 @@ databaseChangeLog:
                   type: text
                   constraints:
                     nullable: true
+              - column:
+                  name: active
+                  type: boolean
+                  defaultValueBoolean: true
+                  constraints:
+                    nullable: false
               - column:
                   name: definition
                   type: text

src/metabase/api/segment.clj

@@ -40,6 +40,7 @@
    definition       [Required Dict]}
   (check-superuser)
   (check-404 (db/exists? Segment :id id))
+  ;; TODO: segment must be active
   (segment/update-segment {:id          id
                            :name        name
                            :description description
@@ -51,7 +52,8 @@
   [id]
   (check-superuser)
   (let-404 [segment (db/sel :one Segment :id id)]
-    (segment/delete-segment id)))
+    (when-not (:active segment)
+      (segment/delete-segment id))))
 
 
 (define-routes)

src/metabase/models/segment.clj

@@ -27,9 +27,6 @@
    (types :definition :json)
    timestamped]
 
-  (post-insert [_ segment]
-    (events/publish-event :segment-create segment))
-
   (post-select [_ {:keys [creator_id description] :as segment}]
     (map->SegmentInstance
       (assoc segment
@@ -54,13 +51,15 @@
          (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)))
+  (let [segment (db/ins Segment
+                  :table_id    table-id
+                  :creator_id  creator-id
+                  :name        name
+                  :description description
+                  :active      true
+                  :definition  definition)]
+    (-> (events/publish-event :segment-create segment)
+        (hydrate :creator))))
 
 (defn retrieve-segment
   "Fetch a single `Segment` by its ID value."

test/metabase/models/segment_test.clj

 (ns metabase.models.segment-test
   (:require [clojure.tools.macro :refer [symbol-macrolet]]
             [expectations :refer :all]
-            [metabase.db :as db]
             (metabase.models [database :refer [Database]]
                              [hydrate :refer :all]
                              [segment :refer :all]
@@ -15,19 +14,21 @@
   (-> (fetch-user username)
       (dissoc :date_joined :last_login)))
 
+(defn segment-details
+  [{:keys [creator] :as segment}]
+  (-> segment
+      (dissoc :id :table_id :created_at :updated_at)
+      (assoc :creator (dissoc creator :date_joined :last_login))))
+
 (defn create-segment-then-select
   [table name description creator definition]
-  (let [{:keys [creator] :as segment} (create-segment table name description creator definition)]
-    (-> segment
-        (dissoc :id :table_id :created_at :updated_at)
-        (assoc :creator (dissoc creator :date_joined :last_login)))))
+  (-> (create-segment table name description creator definition)
+      segment-details))
 
 (defn update-segment-then-select
   [segment message]
-  (let [{:keys [creator] :as segment} (update-segment segment message)]
-    (-> segment
-        (dissoc :id :table_id :created_at :updated_at)
-        (assoc :creator (dissoc creator :date_joined :last_login)))))
+  (-> (update-segment segment message)
+      segment-details))
 
 
 ;; create-segment
@@ -36,6 +37,7 @@
    :creator     (user-details :rasta)
    :name        "I only want *these* things"
    :description nil
+   :active      true
    :definition  {:clause ["a" "b"]}}
   (tu/with-temp Database [{database-id :id} {:name      "Hillbilly"
                                              :engine    :yeehaw
@@ -53,6 +55,7 @@
    :creator      (user-details :rasta)
    :name         "Ivory Tower"
    :description  "All the glorious things..."
+   :active       true
    :definition   {:database 45
                   :query    {:filter ["yay"]}}}
   (tu/with-temp Database [{database-id :id} {:name      "Hillbilly"
@@ -80,6 +83,7 @@
    :creator      (user-details :rasta)
    :name         "Segment 1"
    :description  nil
+   :active       true
    :definition   {}}
   (tu/with-temp Database [{database-id :id} {:name      "Hillbilly"
                                              :engine    :yeehaw
@@ -121,6 +125,7 @@
    :creator      (user-details :rasta)
    :name         "Tatooine"
    :description  nil
+   :active       true
    :definition   {:database 2
                   :query    {:filter ["not" "the droids you're looking for"]}}}
   (tu/with-temp Database [{database-id :id} {:name      "Hillbilly"
@@ -143,4 +148,25 @@
                                      :definition  {:database 2
                                                    :query    {:filter ["not" "the droids you're looking for"]}}} "Just horsing around")))))
 
-;; TODO: delete-segment
+;; delete-segment
+(expect
+  {:creator_id   (user->id :rasta)
+   :creator      (user-details :rasta)
+   :name         "Droids in the desert"
+   :description  "Lookin' for a jedi"
+   :active       false
+   :definition   {}}
+  (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}]
+      (tu/with-temp Segment [{:keys [id]} {:creator_id  (user->id :rasta)
+                                           :table_id    id
+                                           :name        "Droids in the desert"
+                                           :description "Lookin' for a jedi"
+                                           :definition  {}}]
+        (delete-segment id)
+        (segment-details (retrieve-segment id))))))