diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj index 996a5fd79bb57ebf7ee3ca0f148e5f9c0dec0b53..b558bebc68482e2c9072942f856b1a82642b382c 100644 --- a/src/metabase/models/card.clj +++ b/src/metabase/models/card.clj @@ -409,39 +409,40 @@ - card is archived - card.result_metadata changes and the parameter values source field can't be found anymore" [{id :id, :as changes}] - (let [parameter-cards (t2/select ParameterCard :card_id id)] - (doseq [[[po-type po-id] param-cards] - (group-by (juxt :parameterized_object_type :parameterized_object_id) parameter-cards)] - (let [model (case po-type :card 'Card :dashboard 'Dashboard) - {:keys [parameters]} (t2/select-one [model :parameters] :id po-id) - affected-param-ids-set (cond - ;; update all parameters that use this card as source - (:archived changes) - (set (map :parameter_id param-cards)) - - ;; update only parameters that have value_field no longer in this card - (:result_metadata changes) - (let [param-id->parameter (m/index-by :id parameters)] - (->> param-cards - (filter (fn [param-card] - ;; if cant find the value-field in result_metadata, then we should - ;; remove it - (nil? (qp.util/field->field-info + (when (some #{:archived :result_metadata} (keys changes)) + (let [parameter-cards (t2/select ParameterCard :card_id id)] + (doseq [[[po-type po-id] param-cards] + (group-by (juxt :parameterized_object_type :parameterized_object_id) parameter-cards)] + (let [model (case po-type :card 'Card :dashboard 'Dashboard) + {:keys [parameters]} (t2/select-one [model :parameters] :id po-id) + affected-param-ids-set (cond + ;; update all parameters that use this card as source + (:archived changes) + (set (map :parameter_id param-cards)) + + ;; update only parameters that have value_field no longer in this card + (:result_metadata changes) + (let [param-id->parameter (m/index-by :id parameters)] + (->> param-cards + (filter (fn [param-card] + ;; if cant find the value-field in result_metadata, then we should + ;; remove it + (nil? (qp.util/field->field-info (get-in (param-id->parameter (:parameter_id param-card)) [:values_source_config :value_field]) (:result_metadata changes))))) - (map :parameter_id) - set)) - - :else #{}) - new-parameters (map (fn [parameter] - (if (affected-param-ids-set (:id parameter)) - (-> parameter - (assoc :values_source_type nil) - (dissoc :values_source_config)) - parameter)) - parameters)] - (when-not (= parameters new-parameters) - (t2/update! model po-id {:parameters new-parameters})))))) + (map :parameter_id) + set)) + + :else #{}) + new-parameters (map (fn [parameter] + (if (affected-param-ids-set (:id parameter)) + (-> parameter + (assoc :values_source_type nil) + (dissoc :values_source_config)) + parameter)) + parameters)] + (when-not (= parameters new-parameters) + (t2/update! model po-id {:parameters new-parameters}))))))) (defn model-supports-implicit-actions? "A model with implicit action supported iff they are a raw table, @@ -506,6 +507,10 @@ (params/assert-valid-parameters changes) (params/assert-valid-parameter-mappings changes) (update-parameters-using-card-as-values-source changes) + ;; TODO: should be done in after-update + ;; has to place it here because changes is not available in after-update hook see toucan2#129 + (when (contains? changes :dataset_query) + (query-analysis/analyze-async! id)) (when (:parameters changes) (parameter-card/upsert-or-delete-from-parameters! "card" id (:parameters changes))) ;; additional checks (Enterprise Edition only) @@ -558,12 +563,6 @@ maybe-populate-initially-published-at (dissoc :id))) -(t2/define-after-update :model/Card - [card] - (u/prog1 card - (when (contains? (t2/changes card) :dataset_query) - (query-analysis/analyze-async! card)))) - ;; Cards don't normally get deleted (they get archived instead) so this mostly affects tests (t2/define-before-delete :model/Card [{:keys [id] :as _card}] diff --git a/test/metabase/models/query_analysis_test.clj b/test/metabase/models/query_analysis_test.clj index 2ac669557467d4e00f2ffccc560745f1fad608cd..e984006e3e29fa7de0787f3a4f35ab1967c94781 100644 --- a/test/metabase/models/query_analysis_test.clj +++ b/test/metabase/models/query_analysis_test.clj @@ -59,7 +59,9 @@ [card-id query] (if (string? query) (t2/update! :model/Card card-id {:dataset_query (mt/native-query {:query query})}) - (t2/update! :model/Card card-id {:dataset_query query}))) + (t2/update! :model/Card card-id {:dataset_query query})) + ;; TODO remove this hack, adjusting the queue design to handle unsaved cards #45460 + (query-analysis/analyze-card! card-id)) ;;;; ;;;; Actual tests