diff --git a/enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj b/enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj
index 03c5d50f27e1ed407dffe0bffaad004c97a94246..f8f5aaaee5012b21b4aefa2ed9e384613c006e6e 100644
--- a/enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj
@@ -45,7 +45,6 @@
metabase.models.emitter.CardEmitterInstance
metabase.models.emitter.DashboardEmitterInstance
metabase.models.emitter.EmitterInstance
- metabase.models.emitter.EmitterActionInstance
metabase.models.field_values.FieldValuesInstance
metabase.models.login_history.LoginHistoryInstance
metabase.models.metric_important_field.MetricImportantFieldInstance
diff --git a/resources/migrations/000_migrations.yaml b/resources/migrations/000_migrations.yaml
index 36999093f56a4d21ace2dad7bde49f0a9bca9e5c..06d9a06d371eac572ffbe1730877d753850c99a0 100644
--- a/resources/migrations/000_migrations.yaml
+++ b/resources/migrations/000_migrations.yaml
@@ -12542,6 +12542,69 @@ databaseChangeLog:
columnNames: action_id
constraintName: pk_http_action
+ - changeSet:
+ id: v45.00-014
+ author: snoe
+ comment: Added 0.45.0 - writeback add action_id to emitter
+ changes:
+ - addColumn:
+ columns:
+ - column:
+ name: action_id
+ type: int
+ remarks: The related action
+ tableName: emitter
+
+ # FK constraint is added separately because deleteCascade doesn't work in addColumn -- see #14321
+ - changeSet:
+ id: v45.00-015
+ author: snoe
+ comment: Added 0.45.0 - writeback add fk for action_id to emitter
+ changes:
+ - addForeignKeyConstraint:
+ baseTableName: emitter
+ baseColumnNames: action_id
+ referencedTableName: action
+ referencedColumnNames: id
+ constraintName: fk_emitter_ref_action_id
+ onDelete: CASCADE
+
+ - changeSet:
+ id: v45.00-016
+ author: snoe
+ comment: Added 0.45.0 - writeback Move foreign keys from emitter_action and dashboard_emitter
+ changes:
+ - sql:
+ sql: >-
+ update emitter set
+ action_id = (select action_id from emitter_action where emitter_id = id);
+
+ - changeSet:
+ id: v45.00-017
+ author: snoe
+ comment: Added 0.45.0 - writeback remove any emitters that were orphaned to be safe.
+ changes:
+ - sql:
+ sql: delete from emitter where action_id is null;
+
+ - changeSet:
+ id: v45.00-018
+ author: snoe
+ comment: Added 0.45.0 - writeback Make emitter action_id not nullable
+ changes:
+ - addNotNullConstraint:
+ columnDataType: int
+ tableName: emitter
+ columnName: action_id
+
+ - changeSet:
+ id: v45.00-019
+ author: snoe
+ comment: Added 0.45.0 - writeback drop emitter_action
+ changes:
+ - dropTable:
+ tableName: emitter_action
+
# >>>>>>>>>> DO NOT ADD NEW MIGRATIONS BELOW THIS LINE! ADD THEM ABOVE <<<<<<<<<<
########################################################################################################################
diff --git a/src/metabase/api/emitter.clj b/src/metabase/api/emitter.clj
index 1bbc5e752ed4854fce3be8e137c3e5ee873eacaa..abdc5f670a5842ae50ef0948817e76e7c35bed65 100644
--- a/src/metabase/api/emitter.clj
+++ b/src/metabase/api/emitter.clj
@@ -4,8 +4,7 @@
[metabase.actions.http-action :as http-action]
[metabase.api.common :as api]
[metabase.mbql.schema :as mbql.s]
- [metabase.models
- :refer [CardEmitter DashboardEmitter Emitter EmitterAction]]
+ [metabase.models :refer [CardEmitter DashboardEmitter Emitter]]
[metabase.query-processor.writeback :as qp.writeback]
[metabase.util :as u]
[metabase.util.i18n :refer [tru]]
@@ -19,9 +18,7 @@
[emitter-id]
(when-let [emitter (->> (db/query {:select [:*]
:from [[Emitter :emitter]]
- :left-join [[EmitterAction :emitter_action]
- [:= :emitter_action.emitter_id :emitter.id]
- ;; TODO -- not 100% sure we need CardEmitter and DashboardEmitter, we'd
+ :left-join [;; TODO -- not 100% sure we need CardEmitter and DashboardEmitter, we'd
;; only need that if we wanted to hydrate the Card or Dashboard they
;; came from.
[CardEmitter :card_emitter]
@@ -44,8 +41,9 @@
;; Create stuff the hard way by hand because H2 doesn't return the Emitter ID if you have Toucan `pre-insert` create
;; them for you because of some issue with INSERT RETURNING ROWS or something like that. See
;; [[metabase.models.emitter/pre-insert]] for more discussion.
- (let [emitter-id (u/the-id (db/insert! Emitter {:options options, :parameter_mappings parameter_mappings}))]
- (db/insert! EmitterAction {:emitter_id emitter-id, :action_id action_id})
+ (let [emitter-id (u/the-id (db/insert! Emitter {:options options,
+ :parameter_mappings parameter_mappings
+ :action_id action_id}))]
(cond
dashboard_id
(db/insert! DashboardEmitter {:emitter_id emitter-id, :dashboard_id dashboard_id})
diff --git a/src/metabase/models.clj b/src/metabase/models.clj
index 8497ebb1eb97a1750b09dbdb787373ddd9b77c84..28b184c699969f4e87173d1a95a78309f0b0c749 100644
--- a/src/metabase/models.clj
+++ b/src/metabase/models.clj
@@ -105,7 +105,6 @@
[emitter CardEmitter]
[emitter DashboardEmitter]
[emitter Emitter]
- [emitter EmitterAction]
[field Field]
[field-values FieldValues]
[login-history LoginHistory]
diff --git a/src/metabase/models/action.clj b/src/metabase/models/action.clj
index 5546f0af655f062b8ae0ae38e41c4796a264f607..6db464fb69527951a3ddeeabd9c5ccfc10298133 100644
--- a/src/metabase/models/action.clj
+++ b/src/metabase/models/action.clj
@@ -132,10 +132,8 @@
[emitters]
;; emitters apparently might actually be `[nil]` (not 100% sure why) so just make sure we're not doing anything dumb
;; if this is the case.
- (if-let [emitter-ids (seq (filter some? (map :id emitters)))]
- (let [emitter-actions (db/select 'EmitterAction :emitter_id [:in emitter-ids])
- action-id-by-emitter-id (into {} (map (juxt :emitter_id :action_id) emitter-actions))
- actions (m/index-by :id (select-actions :id [:in (map :action_id emitter-actions)]))]
+ (if-let [action-id-by-emitter-id (not-empty (into {} (map (juxt :id :action_id) (filter :id emitters))))]
+ (let [actions-by-id (m/index-by :id (select-actions :id [:in (map val action-id-by-emitter-id)]))]
(for [{emitter-id :id, :as emitter} emitters]
- (some-> emitter (assoc :action (get actions (get action-id-by-emitter-id emitter-id))))))
+ (some-> emitter (assoc :action (get actions-by-id (get action-id-by-emitter-id emitter-id))))))
emitters))
diff --git a/src/metabase/models/emitter.clj b/src/metabase/models/emitter.clj
index 867f56be756da8856a1f9146796e89ceeab265ba..3cc604b4fc00b1d014f926a647d35f7dc238b70b 100644
--- a/src/metabase/models/emitter.clj
+++ b/src/metabase/models/emitter.clj
@@ -12,7 +12,6 @@
(models/defmodel CardEmitter :card_emitter)
(models/defmodel DashboardEmitter :dashboard_emitter)
(models/defmodel Emitter :emitter)
-(models/defmodel EmitterAction :emitter_action)
(defn- normalize-parameter-mappings
[parameter-mappings]
@@ -33,58 +32,11 @@
:options :json})
:properties (constantly {:timestamped? true})}))
-(u/strict-extend
- (class EmitterAction)
- models/IModel
- (merge models/IModelDefaults
- ;; This is ok as long as we're 1:1
- {:primary-key (constantly :emitter_id)}))
-
-(defn- pre-insert
- "We currently support two ways of creating a CardEmitter or DashboardEmitter:
-
- 1. Automatically create the base `Emitter` and `EmitterAction` at the same time. Pass in `:action_id` but not
- `:emitter_id`. This was the original way we coded this but it actually causes a few issues since H2 doesn't seem
- to return the newly created object when you do things this way
-
- 2. Create the base `Emitter` and `EmitterAction` by hand. Pass in `:emitter_id` but not `:action_id`. This is
- probably preferable going forward because H2 does the right thing when you do it this way."
- [{action-id :action_id, emitter-id :emitter_id, :as emitter}]
- ;; if `emitter_id` was passed then assume the Base `Emitter` was created manually. Otherwise, create one
- ;; automatically.
- (let [base-emitter (if emitter-id
- (db/select-one Emitter :id emitter-id)
- (db/insert! Emitter (select-keys emitter [:parameter_mappings :options])))]
- ;; if `action_id` was passed then automatically create an `EmitterAction`. Otherwise assume it was created manually.
- (when action-id
- (db/insert! EmitterAction {:action_id action-id, :emitter_id (u/the-id base-emitter)}))
- ;; ok now Toucan should be able to create the `CardEmitter`/`DashboardEmitter` for us.
- (-> emitter
- (select-keys [:dashboard_id :card_id])
- (assoc :emitter_id (u/the-id base-emitter)))))
-
-(defn- pre-update
- [emitter]
- (when-some [base-emitter (not-empty (select-keys emitter [:parameter_mappings :options]))]
- (db/update! Emitter (:emitter_id emitter) base-emitter))
- (when-some [emitter-action (-> emitter
- (select-keys [:action_id])
- not-empty)]
- (db/update! EmitterAction (:emitter_id emitter) emitter-action))
- (not-empty (select-keys emitter [:emitter_id :dashboard_id :card_id])))
-
-(defn- pre-delete
- [emitter]
- (db/delete! Emitter :id (:emitter_id emitter))
- emitter)
-
(def ^:private Emitter-subtype-IModel-impl
"[[models/IModel]] impl for `DashboardEmitter` and `CardEmitter`"
(merge models/IModelDefaults
- {:primary-key (constantly :emitter_id) ; This is ok as long as we're 1:1
- :pre-delete pre-delete
- :pre-update pre-update
- :pre-insert pre-insert}))
+ ; This is ok as long as we're 1:1
+ {:primary-key (constantly :emitter_id)}))
(u/strict-extend (class DashboardEmitter)
models/IModel
@@ -107,6 +59,7 @@
(let [qualified-join-column (keyword (str "typed_emitter." (name join-column)))
emitters (->> {:select [qualified-join-column
:emitter.id
+ :emitter.action_id
:emitter.options
:emitter.parameter_mappings
:emitter.created_at
diff --git a/test/metabase/actions/test_util.clj b/test/metabase/actions/test_util.clj
index e2058cf17b91246f10e868913aee4e5d1e9be632..f8860662d66864dfa425ada7991edfdbb41c417c 100644
--- a/test/metabase/actions/test_util.clj
+++ b/test/metabase/actions/test_util.clj
@@ -5,7 +5,7 @@
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
[metabase.http-client :as client]
[metabase.models :refer [Action Card CardEmitter Dashboard DashboardEmitter
- Database Emitter EmitterAction QueryAction]]
+ Database Emitter QueryAction]]
[metabase.models.action :as action]
[metabase.test :as mt]
[metabase.test.data.dataset-definitions :as defs]
@@ -168,7 +168,8 @@
(let [parent-model (db/resolve-model card-or-dashboard-model)]
(mt/with-temp* [parent-model [{emitter-parent-id :id}]
Emitter [{emitter-id :id} {:parameter_mappings {"my_id" [:variable [:template-tag "id"]]
- "my_fail" [:variable [:template-tag "fail"]]}}]]
+ "my_fail" [:variable [:template-tag "fail"]]}
+ :action_id action-id}]]
(testing "Sanity check: emitter-id should be non-nil"
(is (integer? emitter-id)))
(testing "Sanity check: make sure parameter mappings were defined the way we'd expect"
@@ -177,8 +178,6 @@
(db/select-one-field :parameter_mappings Emitter :id emitter-id))))
;; these are tied to the Card or Dashboad and Emitter above and will get cascade deleted. We can't use `with-temp*` for them
;; because it doesn't seem to work with tables with compound PKs
- (db/insert! EmitterAction {:emitter_id emitter-id
- :action_id action-id})
(condp = (type parent-model)
(type Card)
(db/insert! CardEmitter {:card_id emitter-parent-id
diff --git a/test/metabase/api/card_test.clj b/test/metabase/api/card_test.clj
index 59259b349001b656a6c36d6068ee01f0e622dc98..ff3880933dc000de54f274840df6ebb9737dfe38 100644
--- a/test/metabase/api/card_test.clj
+++ b/test/metabase/api/card_test.clj
@@ -19,6 +19,7 @@
Collection
Dashboard
Database
+ Emitter
ModerationReview
PersistedInfo
Pulse
@@ -754,8 +755,9 @@
(testing "GET /api/card/:id"
(testing "Fetch card with an emitter"
(mt/with-temp* [Card [read-card {:name "Test Read Card"}]
- Card [write-card {:is_write true :name "Test Write Card"}]]
- (db/insert! CardEmitter {:action_id (u/the-id (db/select-one-field :action_id QueryAction :card_id (u/the-id write-card)))
+ Card [write-card {:is_write true :name "Test Write Card"}]
+ Emitter [{emitter-id :id} {:action_id (u/the-id (db/select-one-field :action_id QueryAction :card_id (u/the-id write-card)))}]]
+ (db/insert! CardEmitter {:emitter_id emitter-id
:card_id (u/the-id read-card)})
(testing "admin sees emitters"
(is (partial=
diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj
index b7008af020cd5cdc3ac23499a5a93b00fc38f692..2159615ba9032af0e63f37c6722990a6b21f8bc4 100644
--- a/test/metabase/api/dashboard_test.clj
+++ b/test/metabase/api/dashboard_test.clj
@@ -15,6 +15,7 @@
DashboardCard
DashboardCardSeries
DashboardEmitter
+ Emitter
Field
FieldValues
Pulse
@@ -345,8 +346,9 @@
(testing "GET /api/dashboard/:id"
(testing "Fetch dashboard with an emitter"
(mt/with-temp* [Dashboard [dashboard {:name "Test Dashboard"}]
- Card [write-card {:is_write true :name "Test Write Card"}]]
- (db/insert! DashboardEmitter {:action_id (u/the-id (db/select-one-field :action_id QueryAction :card_id (u/the-id write-card)))
+ Card [write-card {:is_write true :name "Test Write Card"}]
+ Emitter [{emitter-id :id} {:action_id (u/the-id (db/select-one-field :action_id QueryAction :card_id (u/the-id write-card)))}]]
+ (db/insert! DashboardEmitter {:emitter_id emitter-id
:dashboard_id (u/the-id dashboard)})
(testing "admin sees emitters"
(is (partial=
diff --git a/test/metabase/cmd/copy_test.clj b/test/metabase/cmd/copy_test.clj
index 2173f8a3c9492dd622bc793f75cf16d93dce8e57..f420566e167404f1b8c37551973333bb8eb94827 100644
--- a/test/metabase/cmd/copy_test.clj
+++ b/test/metabase/cmd/copy_test.clj
@@ -26,7 +26,7 @@
(let [migrated-model-names (set (map :name copy/entities))
;; Models that should *not* be migrated in `load-from-h2`.
models-to-exclude #{"TaskHistory" "Query" "QueryCache" "QueryExecution" "CardFavorite" "DashboardFavorite"
- "Action" "HTTPAction" "QueryAction" "CardEmitter" "DashboardEmitter" "EmitterAction"}
+ "Action" "HTTPAction" "QueryAction" "CardEmitter" "DashboardEmitter"}
all-model-names (set (for [ns u/metabase-namespace-symbols
:when (or (re-find #"^metabase\.models\." (name ns))
(= (name ns) "metabase.db.data-migrations"))