add fx

src/metabase/dashboards_rest/api.clj

@@ -15,6 +15,7 @@
    [metabase.collections.core :as collections]
    [metabase.collections.models.collection :as collection]
    [metabase.collections.models.collection.root :as collection.root]
+   [metabase.dashboards-rest.copy :as dashboards.copy]
    [metabase.dashboards.models.dashboard :as dashboard]
    [metabase.dashboards.models.dashboard-card :as dashboard-card]
    [metabase.dashboards.models.dashboard-tab :as dashboard-tab]
@@ -373,153 +374,6 @@
   [id]
   ((get-dashboard-fn *dashboard-load-id*) id))
 
-(mu/defn- cards-to-copy :- [:map
-                            [:discard [:sequential :any]]
-                            [:copy [:map-of ms/PositiveInt :any]]
-                            [:reference [:map-of ms/PositiveInt :any]]]
-  "Returns a map of which cards we need to copy, which cards we need to reference, and which are not to be copied. The
-  `:copy` and `:reference` keys are maps from id to card. The `:discard` key is a vector of cards which were not
-  copied due to permissions.
-
-  If we're making a deep copy, we copy all cards that we have necessary permissions on. Otherwise, we copy Dashboard
-  Questions (questions stored 'in' the dashboard rather than a collection) and reference the rest (assuming
-  permissions)."
-  [deep-copy? :- ms/MaybeBooleanValue
-   dashcards :- [:sequential :any]]
-  (let [card->cards (fn [{:keys [card series]}] (into [card] series))
-        readable? (fn [card] (and (mi/model card) (mi/can-read? card)))
-        card->decision (fn [parent-card card]
-                         (cond
-                           (or
-                            (not (readable? parent-card))
-                            (not (readable? card))
-                            (:archived card))
-                           :discard
-
-                           (or (:dashboard_id card)
-                               (and deep-copy? (not= :model (:type card))))
-                           :copy
-
-                           :else :reference))
-        split-cards (fn [{:keys [card] :as db-card}]
-                      (let [cards (card->cards db-card)]
-                        (group-by (partial card->decision card) cards)))]
-    (reduce (fn [acc db-card]
-              (let [{:keys [discard copy reference]} (split-cards db-card)]
-                (-> acc
-                    (update :reference merge (m/index-by :id reference))
-                    (update :copy merge (m/index-by :id copy))
-                    (update :discard concat discard))))
-            {:reference {}
-             :copy {}
-             :discard []}
-            (filter :card_id dashcards))))
-
-(defn- maybe-duplicate-cards
-  "Takes a dashboard id, and duplicates the cards both on the dashboard's cards and dashcardseries as necessary.
-
-  Returns a map of {:copied {old-card-id duplicated-card} :uncopied [card]} so that the new dashboard can adjust accordingly.
-
-  If `deep-copy?` is `false`, doesn't copy any cards *except* for Dashboard Questions, which must be copied."
-  [deep-copy? new-dashboard old-dashboard dest-coll-id]
-  (let [same-collection?                 (= (:collection_id old-dashboard) dest-coll-id)
-        {:keys [copy discard reference]} (cards-to-copy deep-copy? (:dashcards old-dashboard))]
-    {:copied     (into {} (for [[id to-copy] copy]
-                            [id (queries/create-card!
-                                 (cond-> to-copy
-                                   true                    (assoc :collection_id dest-coll-id)
-                                   same-collection?        (update :name #(str % " - " (tru "Duplicate")))
-                                   (:dashboard_id to-copy) (assoc :dashboard_id (u/the-id new-dashboard)))
-                                 @api/*current-user*
-                                 ;; creating cards from a transaction. wait until tx complete to signal event
-                                 true
-                                 ;; do not autoplace these cards. we will create the dashboard cards ourselves.
-                                 false)]))
-     :discarded  discard
-     :referenced reference}))
-
-(defn- duplicate-tabs
-  [new-dashboard existing-tabs]
-  (let [new-tab-ids (t2/insert-returning-pks! :model/DashboardTab
-                                              (for [tab existing-tabs]
-                                                (-> tab
-                                                    (assoc :dashboard_id (:id new-dashboard))
-                                                    (dissoc :id :entity_id :created_at :updated_at))))]
-    (zipmap (map :id existing-tabs) new-tab-ids)))
-
-(defn- update-colvalmap-setting
-  "Visualizer dashcards have unique visualization settings which embed column id remapping metadata
-  This function iterates through the `:columnValueMapping` viz setting and updates referenced card ids
-
-  col->val-source can look like:
-  {:COLUMN_2 [{:sourceId 'card:<OLD_CARD_ID>', :originalName 'sum', :name 'COLUMN_2'}], ...}"
-  [col->val-source id->new-card]
-  (let [update-cvm-item (fn [item]
-                          (if-let [source-id (:sourceId item)]
-                            (if-let [[_ card-id] (and (string? source-id)
-                                                      (re-find #"^card:(\d+)$" source-id))]
-                              (if-let [new-card (get id->new-card (Long/parseLong card-id))]
-                                (assoc item :sourceId (str "card:" (:id new-card)))
-                                item)
-                              item)
-                            item))
-        update-cvm      (fn [cvm]
-                          (when (map? cvm)
-                            (update-vals cvm #(mapv update-cvm-item %))))]
-    (update-cvm col->val-source)))
-
-(defn update-cards-for-copy
-  "Update dashcards in a dashboard for copying.
-  If the dashboard has tabs, fix up the tab ids in dashcards to point to the new tabs.
-  Then if shallow copy, return the cards. If deep copy, replace ids with id from the newly-copied cards.
-  If there is no new id, it means user lacked curate permissions for the cards
-  collections and it is omitted."
-  [dashcards id->new-card id->referenced-card id->new-tab-id]
-  (let [dashcards (if (seq id->new-tab-id)
-                    (map #(assoc % :dashboard_tab_id (id->new-tab-id (:dashboard_tab_id %)))
-                         dashcards)
-                    dashcards)]
-    (keep (fn [dashboard-card]
-            (cond
-              (:action_id dashboard-card)
-              nil
-
-              (some-> dashboard-card :visualization_settings :virtual_card :display #{"iframe"})
-              dashboard-card
-
-              ;; text cards need no manipulation
-              (some-> dashboard-card :visualization_settings :virtual_card :display #{"text" "heading"})
-              dashboard-card
-
-              ;; referenced cards need no manipulation
-              (get id->referenced-card (:card_id dashboard-card))
-              dashboard-card
-
-              ;; if we didn't duplicate, it doesn't go in the dashboard
-              (not (get id->new-card (:card_id dashboard-card)))
-              nil
-
-              :else
-              (let [new-id (fn [id]
-                             (-> id id->new-card :id))]
-                (-> dashboard-card
-                    (update :card_id new-id)
-                    (assoc :card (-> dashboard-card :card_id id->new-card))
-                    (m/update-existing :parameter_mappings
-                                       (fn [pms]
-                                         (keep (fn [pm]
-                                                 (m/update-existing pm :card_id new-id))
-                                               pms)))
-                    (m/update-existing :series
-                                       (fn [series]
-                                         (keep (fn [card]
-                                                 (when-let [id' (new-id (:id card))]
-                                                   (assoc card :id id')))
-                                               series)))
-                    (m/update-existing-in [:visualization_settings :visualization :columnValuesMapping]
-                                          update-colvalmap-setting id->new-card)))))
-          dashcards)))
-
 ;; TODO (Cam 2025-11-25) please add a response schema to this API endpoint, it makes it easier for our customers to
 ;; use our API + we will need it when we make auto-TypeScript-signature generation happen
 ;;
@@ -543,48 +397,14 @@
                                        :dashboard_id from-dashboard-id
                                        :archived false)))
                  (deferred-tru "You cannot do a shallow copy of this dashboard because it contains Dashboard Questions."))
-  (let [existing-dashboard (get-dashboard from-dashboard-id)
-        dashboard-data {:name                (or name (:name existing-dashboard))
-                        :description         (or description (:description existing-dashboard))
-                        :parameters          (or (:parameters existing-dashboard) [])
-                        :creator_id          api/*current-user-id*
-                        :collection_id       collection_id
-                        :collection_position collection_position
-                        :width               (:width existing-dashboard)}
-        new-cards      (atom nil)
-        dashboard      (t2/with-transaction [_conn]
-                         ;; Adding a new dashboard at `collection_position` could cause other dashboards in this
-                         ;; collection to change position, check that and fix up if needed
-                         (api/maybe-reconcile-collection-position! dashboard-data)
-                         ;; Ok, now save the Dashboard
-                         (let [dash (first (t2/insert-returning-instances! :model/Dashboard dashboard-data))
-                               {id->new-card :copied
-                                id->referenced-card :referenced
-                                uncopied :discarded}
-                               (maybe-duplicate-cards is_deep_copy dash existing-dashboard collection_id)
-
-                               id->new-tab-id (when-let [existing-tabs (seq (:tabs existing-dashboard))]
-                                                (duplicate-tabs dash existing-tabs))]
-                           (reset! new-cards (vals id->new-card))
-                           (when-let [dashcards (seq (update-cards-for-copy (:dashcards existing-dashboard)
-                                                                            id->new-card
-                                                                            id->referenced-card
-                                                                            id->new-tab-id))]
-                             (api/check-500 (dashboard/add-dashcards! dash dashcards)))
-                           (u/prog1 (cond-> dash
-                                      (seq uncopied)
-                                      (assoc :uncopied uncopied))
-                             (when (collections/moving-into-remote-synced? (:collection_id existing-dashboard)
-                                                                           collection_id)
-                               (collections/check-non-remote-synced-dependencies <>)))))]
-    (analytics/track-event! :snowplow/dashboard
-                            {:event        :dashboard-created
-                             :dashboard-id (u/the-id dashboard)})
-    ;; must signal event outside of tx so cards are visible from other threads
-    (when-let [newly-created-cards (seq @new-cards)]
-      (doseq [card newly-created-cards]
-        (events/publish-event! :event/card-create {:object card :user-id api/*current-user-id*})))
-    (events/publish-event! :event/dashboard-create {:object dashboard :user-id api/*current-user-id*})
+  (let [ctx {:request            {:name name :description description :collection_id collection_id
+                                  :collection_position collection_position :is_deep_copy is_deep_copy}
+             :existing-dashboard (get-dashboard from-dashboard-id)
+             :creator-id         api/*current-user-id*}
+        {:keys [dashboard effects]} (t2/with-transaction [_conn]
+                                      (dashboards.copy/copy-dashboard! ctx dashboards.copy/default-fx))]
+    ;; effects (events, analytics) run outside the tx so newly created rows are visible from other threads
+    (dashboards.copy/run-effects! effects)
     dashboard))
 
 ;;; --------------------------------------------- List public and embeddable dashboards ------------------------------

src/metabase/dashboards_rest/copy.clj

@@ -0,0 +1,247 @@
+(ns metabase.dashboards-rest.copy
+  "Dashboard copy logic, factored as a biff.fx-inspired effects orchestrator.
+
+  The pure decision functions (`cards-to-copy`, `update-cards-for-copy`, ...) take data and return data with
+  no IO. The orchestrator [[copy-dashboard!]] is *linear* and takes its write effects as an injected `fx` map
+  so it can be unit-tested with in-memory stubs (no DB). Tail effects that must run *after* the transaction
+  (events, analytics) are returned as data in `:effects` and executed by [[run-effects!]].
+
+  See [[metabase.dashboards-rest.api]] for the endpoint that wires this to a real transaction + `default-fx`."
+  (:require
+   [medley.core :as m]
+   [metabase.analytics.core :as analytics]
+   [metabase.api.common :as api]
+   [metabase.events.core :as events]
+   [metabase.models.interface :as mi]
+   [metabase.queries.core :as queries]
+   [metabase.util :as u]
+   [metabase.util.i18n :refer [tru]]
+   [metabase.util.malli :as mu]
+   [metabase.util.malli.schema :as ms]
+   [toucan2.core :as t2]))
+
+(set! *warn-on-reflection* true)
+
+;;; ------------------------------------------------ pure decisions ------------------------------------------------
+
+(mu/defn cards-to-copy :- [:map
+                           [:discard [:sequential :any]]
+                           [:copy [:map-of ms/PositiveInt :any]]
+                           [:reference [:map-of ms/PositiveInt :any]]]
+  "Returns a map of which cards we need to copy, which cards we need to reference, and which are not to be copied. The
+  `:copy` and `:reference` keys are maps from id to card. The `:discard` key is a vector of cards which were not
+  copied due to permissions.
+
+  If we're making a deep copy, we copy all cards that we have necessary permissions on. Otherwise, we copy Dashboard
+  Questions (questions stored 'in' the dashboard rather than a collection) and reference the rest (assuming
+  permissions)."
+  [deep-copy? :- ms/MaybeBooleanValue
+   dashcards :- [:sequential :any]]
+  (let [card->cards (fn [{:keys [card series]}] (into [card] series))
+        readable? (fn [card] (and (mi/model card) (mi/can-read? card)))
+        card->decision (fn [parent-card card]
+                         (cond
+                           (or
+                            (not (readable? parent-card))
+                            (not (readable? card))
+                            (:archived card))
+                           :discard
+
+                           (or (:dashboard_id card)
+                               (and deep-copy? (not= :model (:type card))))
+                           :copy
+
+                           :else :reference))
+        split-cards (fn [{:keys [card] :as db-card}]
+                      (let [cards (card->cards db-card)]
+                        (group-by (partial card->decision card) cards)))]
+    (reduce (fn [acc db-card]
+              (let [{:keys [discard copy reference]} (split-cards db-card)]
+                (-> acc
+                    (update :reference merge (m/index-by :id reference))
+                    (update :copy merge (m/index-by :id copy))
+                    (update :discard concat discard))))
+            {:reference {}
+             :copy {}
+             :discard []}
+            (filter :card_id dashcards))))
+
+(defn card-copy-specs
+  "Pure: given the cards we decided to `:copy`, return a seq of `[old-card-id card-data]` pairs describing the cards to
+  create (correct collection, renamed if copying within the same collection, re-parented if a Dashboard Question). The
+  actual create is performed by the `fx` `:create-card!` handler — this just computes *what* to create."
+  [copy new-dashboard-id same-collection? dest-coll-id]
+  (for [[id to-copy] copy]
+    [id (cond-> to-copy
+          true                    (assoc :collection_id dest-coll-id)
+          same-collection?        (update :name #(str % " - " (tru "Duplicate")))
+          (:dashboard_id to-copy) (assoc :dashboard_id new-dashboard-id))]))
+
+(defn tab-copy-specs
+  "Pure: strip db-managed keys off each existing tab and re-parent it to `new-dashboard-id`, ready for insert."
+  [new-dashboard-id existing-tabs]
+  (for [tab existing-tabs]
+    (-> tab
+        (assoc :dashboard_id new-dashboard-id)
+        (dissoc :id :entity_id :created_at :updated_at))))
+
+(defn- update-colvalmap-setting
+  "Visualizer dashcards have unique visualization settings which embed column id remapping metadata
+  This function iterates through the `:columnValueMapping` viz setting and updates referenced card ids
+
+  col->val-source can look like:
+  {:COLUMN_2 [{:sourceId 'card:<OLD_CARD_ID>', :originalName 'sum', :name 'COLUMN_2'}], ...}"
+  [col->val-source id->new-card]
+  (let [update-cvm-item (fn [item]
+                          (if-let [source-id (:sourceId item)]
+                            (if-let [[_ card-id] (and (string? source-id)
+                                                      (re-find #"^card:(\d+)$" source-id))]
+                              (if-let [new-card (get id->new-card (Long/parseLong card-id))]
+                                (assoc item :sourceId (str "card:" (:id new-card)))
+                                item)
+                              item)
+                            item))
+        update-cvm      (fn [cvm]
+                          (when (map? cvm)
+                            (update-vals cvm #(mapv update-cvm-item %))))]
+    (update-cvm col->val-source)))
+
+(defn update-cards-for-copy
+  "Update dashcards in a dashboard for copying.
+  If the dashboard has tabs, fix up the tab ids in dashcards to point to the new tabs.
+  Then if shallow copy, return the cards. If deep copy, replace ids with id from the newly-copied cards.
+  If there is no new id, it means user lacked curate permissions for the cards
+  collections and it is omitted."
+  [dashcards id->new-card id->referenced-card id->new-tab-id]
+  (let [dashcards (if (seq id->new-tab-id)
+                    (map #(assoc % :dashboard_tab_id (id->new-tab-id (:dashboard_tab_id %)))
+                         dashcards)
+                    dashcards)]
+    (keep (fn [dashboard-card]
+            (cond
+              (:action_id dashboard-card)
+              nil
+
+              (some-> dashboard-card :visualization_settings :virtual_card :display #{"iframe"})
+              dashboard-card
+
+              ;; text cards need no manipulation
+              (some-> dashboard-card :visualization_settings :virtual_card :display #{"text" "heading"})
+              dashboard-card
+
+              ;; referenced cards need no manipulation
+              (get id->referenced-card (:card_id dashboard-card))
+              dashboard-card
+
+              ;; if we didn't duplicate, it doesn't go in the dashboard
+              (not (get id->new-card (:card_id dashboard-card)))
+              nil
+
+              :else
+              (let [new-id (fn [id]
+                             (-> id id->new-card :id))]
+                (-> dashboard-card
+                    (update :card_id new-id)
+                    (assoc :card (-> dashboard-card :card_id id->new-card))
+                    (m/update-existing :parameter_mappings
+                                       (fn [pms]
+                                         (keep (fn [pm]
+                                                 (m/update-existing pm :card_id new-id))
+                                               pms)))
+                    (m/update-existing :series
+                                       (fn [series]
+                                         (keep (fn [card]
+                                                 (when-let [id' (new-id (:id card))]
+                                                   (assoc card :id id')))
+                                               series)))
+                    (m/update-existing-in [:visualization_settings :visualization :columnValuesMapping]
+                                          update-colvalmap-setting id->new-card)))))
+          dashcards)))
+
+(defn dashboard-data
+  "Pure: build the row to insert for the copied dashboard, merging request overrides over the original's values."
+  [{:keys [name description collection_id collection_position]} existing-dashboard creator-id]
+  {:name                (or name (:name existing-dashboard))
+   :description         (or description (:description existing-dashboard))
+   :parameters          (or (:parameters existing-dashboard) [])
+   :creator_id          creator-id
+   :collection_id       collection_id
+   :collection_position collection_position
+   :width               (:width existing-dashboard)})
+
+;;; -------------------------------------------------- effects -----------------------------------------------------
+
+(def default-fx
+  "Real write-effect handlers for [[copy-dashboard!]]. Tests pass an alternate map of stubs.
+
+  Each handler performs one DB write and returns the value the orchestrator threads forward:
+    :reconcile-position! -> nil  (side effect: shift sibling collection_position)
+    :insert-dashboard!   -> the inserted dashboard instance
+    :create-card!        -> the created card instance (delay-event?=true: event fired post-tx)
+    :insert-tabs!        -> seq of new tab pks (ordered to match input)
+    :add-dashcards!      -> truthy on success
+    :check-remote-sync!  -> nil  (side effect: throws if dest forbids the deps)"
+  {:reconcile-position! (fn [dashboard-data] (api/maybe-reconcile-collection-position! dashboard-data))
+   :insert-dashboard!   (fn [dashboard-data] (first (t2/insert-returning-instances! :model/Dashboard dashboard-data)))
+   :create-card!        (fn [card-data] (queries/create-card! card-data @api/*current-user* true false))
+   :insert-tabs!        (fn [tab-specs] (t2/insert-returning-pks! :model/DashboardTab tab-specs))
+   :add-dashcards!      (fn [dashboard dashcards] ((requiring-resolve 'metabase.dashboards.models.dashboard/add-dashcards!) dashboard dashcards))
+   :check-remote-sync!  (fn [existing-dashboard new-coll-id new-dashboard]
+                          (let [collections (requiring-resolve 'metabase.collections.core/moving-into-remote-synced?)
+                                check       (requiring-resolve 'metabase.collections.core/check-non-remote-synced-dependencies)]
+                            (when (collections (:collection_id existing-dashboard) new-coll-id)
+                              (check new-dashboard))))})
+
+(defn run-effects!
+  "Execute the tail `:effects` data returned by [[copy-dashboard!]]. These run *after* the transaction so newly
+  created rows are visible from other threads (events) — mirroring the prior inline behaviour."
+  [effects]
+  (doseq [[k & args] effects]
+    (case k
+      :event     (apply events/publish-event! args)
+      :analytics (apply analytics/track-event! args))))
+
+;;; ----------------------------------------------- orchestrator ---------------------------------------------------
+
+(defn copy-dashboard!
+  "Linear orchestrator for copying a dashboard. Performs its write effects through the injected `fx` map (default
+  [[default-fx]]) so it can be exercised with in-memory stubs and no database.
+
+  `ctx` keys:
+    :request            - the request body map (:name :description :collection_id :collection_position :is_deep_copy)
+    :existing-dashboard - the fully-hydrated source dashboard (with :dashcards and :tabs)
+    :creator-id         - id of the user performing the copy
+
+  Returns `{:dashboard <copied dashboard, with :uncopied attached if any cards were dropped>
+            :effects   <vector of [:event ...] / [:analytics ...] tuples to run after the tx>}`.
+
+  The data-dependency chain (insert dashboard -> its id -> copy cards keyed by new id -> build dashcards) lives in
+  ordinary `let` bindings — no state machine. The caller wraps this in a transaction and calls [[run-effects!]]."
+  [{:keys [request existing-dashboard creator-id]} fx]
+  (let [{:keys [collection_id is_deep_copy]} request
+        new-row          (dashboard-data request existing-dashboard creator-id)
+        same-collection? (= (:collection_id existing-dashboard) collection_id)
+        ;; Adding a new dashboard at collection_position could shift siblings; fix up first.
+        _                ((:reconcile-position! fx) new-row)
+        dash             ((:insert-dashboard! fx) new-row)
+        ;; decide (pure) which cards to copy/reference/discard, then create the copies (effect, feedback: new ids)
+        {:keys [copy reference discard]} (cards-to-copy is_deep_copy (:dashcards existing-dashboard))
+        id->new-card     (into {} (for [[old-id card-data] (card-copy-specs copy (u/the-id dash) same-collection? collection_id)]
+                                    [old-id ((:create-card! fx) card-data)]))
+        new-cards        (vals id->new-card)
+        ;; tabs: pure spec -> insert -> zip old->new ids (feedback used when rewriting dashcards)
+        existing-tabs    (seq (:tabs existing-dashboard))
+        id->new-tab-id   (when existing-tabs
+                           (zipmap (map :id existing-tabs)
+                                   ((:insert-tabs! fx) (tab-copy-specs (:id dash) existing-tabs))))
+        ;; pure: rewrite dashcards to point at the new cards/tabs, then insert them
+        dashcards        (seq (update-cards-for-copy (:dashcards existing-dashboard) id->new-card reference id->new-tab-id))
+        _                (when dashcards
+                           (api/check-500 ((:add-dashcards! fx) dash dashcards)))
+        dashboard        (cond-> dash (seq discard) (assoc :uncopied discard))]
+    ((:check-remote-sync! fx) existing-dashboard collection_id dashboard)
+    {:dashboard dashboard
+     :effects   (-> [[:analytics :snowplow/dashboard {:event :dashboard-created :dashboard-id (u/the-id dashboard)}]]
+                    (into (for [card new-cards]
+                            [:event :event/card-create {:object card :user-id creator-id}]))
+                    (conj [:event :event/dashboard-create {:object dashboard :user-id creator-id}]))}))

test/metabase/dashboards_rest/api_test.clj

@@ -15,6 +15,7 @@
    [metabase.collections.models.collection :as collection]
    [metabase.config.core :as config]
    [metabase.dashboards-rest.api :as api.dashboard]
+   [metabase.dashboards-rest.copy :as dashboards.copy]
    [metabase.dashboards.models.dashboard-card :as dashboard-card]
    [metabase.dashboards.models.dashboard-test :as dashboard-test]
    [metabase.driver :as driver]
@@ -67,7 +68,7 @@
                            :COLUMN_4 [{:sourceId "not-a-card" :originalName "x" :name "COLUMN_4"}]
                            :COLUMN_5 [{:sourceId "card:abc" :originalName "invalid" :name "COLUMN_5"}]
                            :COLUMN_6 [{:name "No source ID"}]}
-          result (#'api.dashboard/update-colvalmap-setting col->val-source id->new-card)]
+          result (#'dashboards.copy/update-colvalmap-setting col->val-source id->new-card)]
       (testing "should update valid card IDs that exist in the map"
         (is (= "card:456" (-> result :COLUMN_1 first :sourceId)))
         (is (= "card:987" (-> result :COLUMN_2 first :sourceId))))
@@ -1590,7 +1591,7 @@
         (is (= {:copy {1 {:id 1} 2 {:id 2} 3 {:id 3}}
                 :reference {}
                 :discard []}
-               (#'api.dashboard/cards-to-copy true dashcards))))))
+               (dashboards.copy/cards-to-copy true dashcards))))))
   (testing "Identifies cards which cannot be copied"
     (testing "If they are in a series"
       (let [dashcards [{:card_id 1 :card (card-model {:id 1}) :series [(card-model {:id 2})]}
@@ -1599,7 +1600,7 @@
           (is (= {:copy {1 {:id 1} 3 {:id 3}}
                   :reference {}
                   :discard [{:id 2}]}
-                 (#'api.dashboard/cards-to-copy true dashcards))))))
+                 (dashboards.copy/cards-to-copy true dashcards))))))
     (testing "When the base of a series lacks permissions"
       (let [dashcards [{:card_id 1 :card (card-model {:id 1}) :series [(card-model {:id 2})]}
                        {:card_id 3 :card (card-model {:id 3})}]]
@@ -1607,7 +1608,7 @@
           (is (= {:copy {3 {:id 3}}
                   :reference {}
                   :discard [{:id 1} {:id 2}]}
-                 (#'api.dashboard/cards-to-copy true dashcards)))))))
+                 (dashboards.copy/cards-to-copy true dashcards)))))))
   (testing "Identifies cards to be referenced"
     (let [dashcards [{:card_id 1 :card (card-model {:id 1}) :series [(card-model {:id 2})]}
                      {:card_id 3 :card (card-model {:id 3})}]]
@@ -1617,7 +1618,7 @@
                             3 {:id 3}}
                 :copy {}
                 :discard []}
-               (#'api.dashboard/cards-to-copy false dashcards))))))
+               (dashboards.copy/cards-to-copy false dashcards))))))
   (testing "Identifies cards that cannot be referenced"
     (let [dashcards [{:card_id 1 :card (card-model {:id 1}) :series [(card-model {:id 2})]}
                      {:card_id 3 :card (card-model {:id 3})}]]
@@ -1626,54 +1627,54 @@
                             3 {:id 3}}
                 :copy {}
                 :discard [{:id 2}]}
-               (#'api.dashboard/cards-to-copy false dashcards)))))))
+               (dashboards.copy/cards-to-copy false dashcards)))))))
 
 (deftest update-cards-for-copy-test
   (testing "Returns the original dashcards for referenced dashcards"
     (let [dashcards [{:card_id 1 :card {:id 1} :series [{:id 2}]}
                      {:card_id 3 :card {:id 3}}]]
       (is (= dashcards
-             (api.dashboard/update-cards-for-copy dashcards
-                                                  nil
-                                                  {1 {:id 1}
-                                                   2 {:id 2}
-                                                   3 {:id 3}}
-                                                  nil))))
+             (dashboards.copy/update-cards-for-copy dashcards
+                                                    nil
+                                                    {1 {:id 1}
+                                                     2 {:id 2}
+                                                     3 {:id 3}}
+                                                    nil))))
     (testing "with tab-ids updated if dashboard has tab"
       (is (= [{:card_id 1 :card {:id 1} :dashboard_tab_id 10}
               {:card_id 3 :card {:id 3} :dashboard_tab_id 20}]
-             (api.dashboard/update-cards-for-copy [{:card_id 1 :card {:id 1} :dashboard_tab_id 1}
-                                                   {:card_id 3 :card {:id 3} :dashboard_tab_id 2}]
-                                                  nil
-                                                  {1 {:id 1}
-                                                   2 {:id 2}
-                                                   3 {:id 3}}
-                                                  {1 10
-                                                   2 20}))))))
+             (dashboards.copy/update-cards-for-copy [{:card_id 1 :card {:id 1} :dashboard_tab_id 1}
+                                                     {:card_id 3 :card {:id 3} :dashboard_tab_id 2}]
+                                                    nil
+                                                    {1 {:id 1}
+                                                     2 {:id 2}
+                                                     3 {:id 3}}
+                                                    {1 10
+                                                     2 20}))))))
 
 (deftest update-cards-for-copy-test-2
   (testing "When copy style is deep"
     (let [dashcards [{:card_id 1 :card {:id 1} :series [{:id 2} {:id 3}]}]]
       (testing "Can omit series cards"
         (is (= [{:card_id 5 :card {:id 5} :series [{:id 6}]}]
-               (api.dashboard/update-cards-for-copy dashcards
-                                                    {1 {:id 5}
-                                                     2 {:id 6}}
-                                                    nil
-                                                    nil)))))
+               (dashboards.copy/update-cards-for-copy dashcards
+                                                      {1 {:id 5}
+                                                       2 {:id 6}}
+                                                      nil
+                                                      nil)))))
     (testing "Can omit whole card with series if not copied"
       (let [dashcards [{:card_id 1 :card {} :series [{:id 2} {:id 3}]}
                        {:card_id 4 :card {} :series [{:id 5} {:id 6}]}]]
         (is (= [{:card_id 7 :card {:id 7} :series [{:id 8} {:id 9}]}]
-               (api.dashboard/update-cards-for-copy dashcards
-                                                    {1 {:id 7}
-                                                     2 {:id 8}
-                                                     3 {:id 9}
-                                                     ;; not copying id 4 which is the base of the following two
-                                                     5 {:id 10}
-                                                     6 {:id 11}}
-                                                    nil
-                                                    nil)))))
+               (dashboards.copy/update-cards-for-copy dashcards
+                                                      {1 {:id 7}
+                                                       2 {:id 8}
+                                                       3 {:id 9}
+                                                       ;; not copying id 4 which is the base of the following two
+                                                       5 {:id 10}
+                                                       6 {:id 11}}
+                                                      nil
+                                                      nil)))))
     (testing "Updates parameter mappings to new card ids"
       (let [dashcards [{:card_id            1
                         :card               {:id 1}
@@ -1687,10 +1688,10 @@
                                        :card_id      2
                                        :target       [:dimension
                                                       [:field 63 nil]]}]}]
-               (api.dashboard/update-cards-for-copy dashcards
-                                                    {1 {:id 2}}
-                                                    nil
-                                                    nil)))))
+               (dashboards.copy/update-cards-for-copy dashcards
+                                                      {1 {:id 2}}
+                                                      nil
+                                                      nil)))))
     (testing "Does not think action cards are text cards"
       (let [dashcards [{:card_id 1 :card {:id 1}}
                        {:visualization_settings {:virtual_card {:display "text"}
@@ -1699,20 +1700,20 @@
                                                  :text         "keep me!"}}
                        {:action_id 123}]]
         (is (= (butlast dashcards)
-               (api.dashboard/update-cards-for-copy dashcards
-                                                    {1 {:id 1}}
-                                                    nil
-                                                    nil)))))
+               (dashboards.copy/update-cards-for-copy dashcards
+                                                      {1 {:id 1}}
+                                                      nil
+                                                      nil)))))
     (testing "Copies iframe cards"
       (let [dashcards [{:card_id 1 :card {:id 1}}
                        {:visualization_settings
                         {:virtual_card {:display "iframe"}
                          :iframe "<iframe src=\"https://www.youtube.com/embed/dQw4w9WgXcQ\" />"}}]]
         (is (= dashcards
-               (api.dashboard/update-cards-for-copy dashcards
-                                                    {1 {:id 1}}
-                                                    nil
-                                                    nil)))))))
+               (dashboards.copy/update-cards-for-copy dashcards
+                                                      {1 {:id 1}}
+                                                      nil
+                                                      nil)))))))
 
 (deftest copy-dashboard-cards-test
   (testing "POST /api/dashboard/:id/copy"

test/metabase/dashboards_rest/copy_test.clj

@@ -0,0 +1,163 @@
+(ns metabase.dashboards-rest.copy-test
+  "DB-free unit tests for the dashboard-copy orchestrator. The whole point of the biff.fx-inspired `fx`-injection
+  design is that [[metabase.dashboards-rest.copy/copy-dashboard!]] can be exercised with in-memory stub effects -
+  no `with-temp`, no `with-current-user`, no database. These tests assert both the returned data (`:dashboard`,
+  `:effects`) and the exact sequence of write effects, including the insert -> new-id -> dashcards feedback chain."
+  (:require
+   [clojure.test :refer [deftest is testing]]
+   [metabase.analytics.core :as analytics]
+   [metabase.dashboards-rest.copy :as copy]
+   [metabase.events.core :as events]
+   [metabase.models.interface :as mi]))
+
+(set! *warn-on-reflection* true)
+
+;;; ----------------------------------------------- pure helpers ---------------------------------------------------
+
+(deftest ^:parallel dashboard-data-test
+  (testing "request values win over the original; falls back to original when absent"
+    (is (= {:name "New" :description "orig-desc" :parameters [] :creator_id 7
+            :collection_id 3 :collection_position 2 :width "fixed"}
+           (copy/dashboard-data {:name "New" :collection_id 3 :collection_position 2}
+                                {:name "Orig" :description "orig-desc" :width "fixed"}
+                                7))))
+  (testing "nil parameters fall back to an empty vector"
+    (is (= [] (:parameters (copy/dashboard-data {} {} 1))))))
+
+(deftest ^:parallel card-copy-specs-test
+  (testing "every copied card is re-pointed at the destination collection"
+    (is (= [[1 {:id 1 :name "A" :collection_id 99}]]
+           (copy/card-copy-specs {1 {:id 1 :name "A"}} 100 false 99))))
+  (testing "copying within the same collection renames with ' - Duplicate'"
+    (is (= "A - Duplicate"
+           (-> (copy/card-copy-specs {1 {:id 1 :name "A"}} 100 true 99) first second :name))))
+  (testing "Dashboard Questions are re-parented to the new dashboard id"
+    (is (= 100
+           (-> (copy/card-copy-specs {1 {:id 1 :name "A" :dashboard_id 5}} 100 false 99)
+               first second :dashboard_id)))))
+
+(deftest ^:parallel tab-copy-specs-test
+  (testing "db-managed keys are stripped and the tab is re-parented"
+    (is (= [{:name "T1" :dashboard_id 100}]
+           (copy/tab-copy-specs 100 [{:id 9 :entity_id "x" :created_at :t :updated_at :t :name "T1"}])))))
+
+;;; ------------------------------------------- fx stubbing harness ------------------------------------------------
+
+(defn- recording-fx
+  "Returns `[fx calls]` where `fx` is a stub effect map that records every call into the `calls` atom as
+  `[handler-key & args]` and returns deterministic fakes, and `calls` is the atom holding the call log."
+  []
+  (let [calls (atom [])
+        record (fn [k ret-fn]
+                 (fn [& args]
+                   (swap! calls conj (into [k] args))
+                   (ret-fn args)))]
+    [{:reconcile-position! (record :reconcile-position! (constantly nil))
+      ;; insert returns a fixed new dashboard id -> drives the feedback chain
+      :insert-dashboard!   (record :insert-dashboard! (constantly {:id 100 :name "copy"}))
+      ;; each created card gets id = old-id + 1000, so we can assert the id remapping
+      :create-card!        (record :create-card! (fn [[card-data]] {:id (+ 1000 (:id card-data)) :name (:name card-data)}))
+      :insert-tabs!        (record :insert-tabs! (fn [[specs]] (map-indexed (fn [i _] (+ 200 i)) specs)))
+      :add-dashcards!      (record :add-dashcards! (constantly true))
+      :check-remote-sync!  (record :check-remote-sync! (constantly nil))}
+     calls]))
+
+(defn- calls-of [calls k] (filter #(= k (first %)) @calls))
+
+;;; ------------------------------------------- orchestrator tests -------------------------------------------------
+
+;; not ^:parallel — kondo flags copy-dashboard! as destructive (it is not, but the ! suffix triggers the linter)
+(deftest copy-dashboard!-minimal-test
+  (testing "a dashboard with no cards/tabs: inserts the dashboard, adds no dashcards, returns events"
+    (let [[fx calls] (recording-fx)
+          ctx {:request {:name "My copy" :collection_id 3 :is_deep_copy false}
+               :existing-dashboard {:name "Orig" :dashcards [] :tabs []}
+               :creator-id 7}
+          {:keys [dashboard effects]} (copy/copy-dashboard! ctx fx)]
+      (testing "returns the inserted dashboard"
+        (is (= {:id 100 :name "copy"} dashboard)))
+      (testing "inserts exactly one dashboard with merged data"
+        (is (= 1 (count (calls-of calls :insert-dashboard!))))
+        (is (= "My copy" (-> (calls-of calls :insert-dashboard!) first second :name))))
+      (testing "no cards copied, no tabs inserted, no dashcards added"
+        (is (empty? (calls-of calls :create-card!)))
+        (is (empty? (calls-of calls :insert-tabs!)))
+        (is (empty? (calls-of calls :add-dashcards!))))
+      (testing "tail effects are returned as data: analytics + dashboard-create only"
+        (is (= [[:analytics :snowplow/dashboard {:event :dashboard-created :dashboard-id 100}]
+                [:event :event/dashboard-create {:object {:id 100 :name "copy"} :user-id 7}]]
+               effects))))))
+
+;; NOTE: not ^:parallel — these tests with-redef the global mi/can-read? var, which is not thread-safe.
+(deftest copy-dashboard!-deep-copy-feedback-chain-test
+  (testing "deep copy: card is copied (effect), its NEW id flows into the inserted dashcard (feedback chain)"
+    ;; cards-to-copy reads ambient permission state via mi/can-read?; stub it readable for a DB-free test.
+    (with-redefs [mi/can-read? (constantly true)
+                  mi/model (constantly :model/Card)]
+      (let [card {:id 1 :name "Q1" :type :question}
+            ctx {:request {:is_deep_copy true :collection_id 3}
+                 :existing-dashboard {:name "Orig"
+                                      :collection_id 99 ;; different dest -> no rename
+                                      :dashcards [{:card_id 1 :card card}]
+                                      :tabs []}
+                 :creator-id 7}
+            [fx calls] (recording-fx)
+            {:keys [effects]} (copy/copy-dashboard! ctx fx)]
+        (testing "the readable question gets created"
+          (is (= 1 (count (calls-of calls :create-card!)))))
+        (testing "the dashcard added points at the NEW card id (1 -> 1001), proving the feedback chain"
+          (let [[[_ _dash [dashcard]]] (calls-of calls :add-dashcards!)]
+            (is (= 1001 (:card_id dashcard)))))
+        (testing "a :card-create event is emitted for the new card, before the dashboard-create event"
+          (is (= [[:analytics :snowplow/dashboard {:event :dashboard-created :dashboard-id 100}]
+                  [:event :event/card-create {:object {:id 1001 :name "Q1"} :user-id 7}]
+                  [:event :event/dashboard-create {:object {:id 100 :name "copy"} :user-id 7}]]
+                 effects)))))))
+
+;; not ^:parallel — kondo flags copy-dashboard! as destructive (it is not, but the ! suffix triggers the linter)
+(deftest copy-dashboard!-discard-test
+  (testing "unreadable cards are discarded and surfaced as :uncopied on the result dashboard"
+    ;; a plain map card has no model (mi/model -> nil), so cards-to-copy treats it as unreadable -> :discard.
+    ;; This exercises the discard branch with zero redefs, so it stays parallel-safe.
+    (let [ctx {:request {:is_deep_copy true :collection_id 3}
+               :existing-dashboard {:name "Orig" :collection_id 99
+                                    :dashcards [{:card_id 1 :card {:id 1 :type :question}}]
+                                    :tabs []}
+               :creator-id 7}
+          [fx calls] (recording-fx)
+          {:keys [dashboard]} (copy/copy-dashboard! ctx fx)]
+      (testing "nothing gets created and no dashcards are added"
+        (is (empty? (calls-of calls :create-card!)))
+        (is (empty? (calls-of calls :add-dashcards!))))
+      (testing "the discarded card is attached as :uncopied"
+        (is (= [{:id 1 :type :question}] (:uncopied dashboard)))))))
+
+;; NOTE: not ^:parallel — with-redefs of the global mi/can-read? var is not thread-safe.
+(deftest copy-dashboard!-tabs-test
+  (testing "tabs are inserted and the dashcard's tab id is remapped old -> new"
+    (with-redefs [mi/can-read? (constantly true)
+                  mi/model (constantly :model/Card)]
+      (let [ctx {:request {:is_deep_copy true :collection_id 99}
+                 :existing-dashboard {:name "Orig" :collection_id 99
+                                      :tabs [{:id 9 :name "T"}]
+                                      :dashcards [{:card_id 1 :card {:id 1 :type :question} :dashboard_tab_id 9}]}
+                 :creator-id 7}
+            [fx calls] (recording-fx)]
+        (copy/copy-dashboard! ctx fx)
+        (testing "one tab insert happened"
+          (is (= 1 (count (calls-of calls :insert-tabs!)))))
+        (testing "the dashcard's tab id is remapped from old 9 to the new 200"
+          (let [[[_ _dash [dashcard]]] (calls-of calls :add-dashcards!)]
+            (is (= 200 (:dashboard_tab_id dashcard)))))))))
+
+;; not ^:parallel — with-redefs of global event/analytics vars is not thread-safe.
+(deftest run-effects!-test
+  (testing "run-effects! dispatches tuples to the right side-effecting fn"
+    (let [fired (atom [])]
+      (with-redefs [events/publish-event! (fn [& a] (swap! fired conj (into [:event] a)))
+                    analytics/track-event! (fn [& a] (swap! fired conj (into [:analytics] a)))]
+        (copy/run-effects! [[:analytics :snowplow/dashboard {:dashboard-id 100}]
+                            [:event :event/dashboard-create {:object {:id 100}}]])
+        (is (= [[:analytics :snowplow/dashboard {:dashboard-id 100}]
+                [:event :event/dashboard-create {:object {:id 100}}]]
+               @fired))))))