diff --git a/src/metabase/api/dashboard.clj b/src/metabase/api/dashboard.clj
index d8a4d7ac9744d58a67ed83680320bafdb3bf000c..8610a07408e21db7e623b46715f1c6d55cba9279 100644
--- a/src/metabase/api/dashboard.clj
+++ b/src/metabase/api/dashboard.clj
@@ -283,45 +283,60 @@
:uncopied discard}
copy)))
+(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-cards-for-copy
- "Update ordered-cards in a dashboard for copying. If shallow copy, returns the cards. If deep copy, replaces ids with
- id from the newly-copied cards. If there is no new id, it means user lacked curate permissions for the cards
+ "Update ordered-cards in a dashboard for copying.
+ If the dashboard has tabs, fix up the tab ids in ordered-cards 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. Dashboard-id is only needed for useful errors."
- [dashboard-id ordered-cards deep? id->new-card]
+ [dashboard-id ordered-cards deep? id->new-card id->new-tab-id]
(when (and deep? (nil? id->new-card))
(throw (ex-info (tru "No copied card information found")
{:user-id api/*current-user-id*
:dashboard-id dashboard-id})))
- (if-not deep?
- ordered-cards
- (keep (fn [dashboard-card]
- (cond
- ;; text cards need no manipulation
- (nil? (:card_id dashboard-card))
- dashboard-card
-
- ;; if we didn't duplicate, it doesn't go in the dashboard
- (not (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)))))))
- ordered-cards)))
+ (let [ordered-cards (if (seq id->new-tab-id)
+ (map #(assoc % :dashboard_tab_id (id->new-tab-id (:dashboard_tab_id %)))
+ ordered-cards)
+ ordered-cards)]
+ (if-not deep?
+ ordered-cards
+ (keep (fn [dashboard-card]
+ (cond
+ ;; text cards need no manipulation
+ (nil? (:card_id dashboard-card))
+ dashboard-card
+
+ ;; if we didn't duplicate, it doesn't go in the dashboard
+ (not (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)))))))
+ ordered-cards))))
(api/defendpoint POST "/:from-dashboard-id/copy"
"Copy a Dashboard."
@@ -351,12 +366,16 @@
(let [dash (first (t2/insert-returning-instances! :model/Dashboard dashboard-data))
{id->new-card :copied uncopied :uncopied}
(when is_deep_copy
- (duplicate-cards existing-dashboard collection_id))]
+ (duplicate-cards existing-dashboard collection_id))
+
+ id->new-tab-id (when-let [existing-tabs (seq (:ordered_tabs existing-dashboard))]
+ (duplicate-tabs dash existing-tabs))]
(reset! new-cards (vals id->new-card))
(when-let [dashcards (seq (update-cards-for-copy from-dashboard-id
(:ordered_cards existing-dashboard)
is_deep_copy
- id->new-card))]
+ id->new-card
+ id->new-tab-id))]
(api/check-500 (dashboard/add-dashcards! dash dashcards)))
(cond-> dash
(seq uncopied)
diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj
index 58c7e9324d1ab417ad33110a02707761b36ecf82..aa7736fd67ee3f2b9650bdcb68edbe511f61a199 100644
--- a/test/metabase/api/dashboard_test.clj
+++ b/test/metabase/api/dashboard_test.clj
@@ -127,6 +127,38 @@
(defmacro ^:private with-dashboards-in-writeable-collection [dashboards-or-ids & body]
`(do-with-dashboards-in-a-collection perms/grant-collection-readwrite-permissions! ~dashboards-or-ids (fn [] ~@body)))
+(defn- do-with-simple-dashboard-with-tabs
+ [f]
+ (t2.with-temp/with-temp
+ [Dashboard {dashboard-id :id} {}
+
+ Card {card-id-1 :id} {}
+
+ Card {card-id-2 :id} {}
+ :model/DashboardTab {dashtab-id-1 :id} {:name "Tab 1"
+ :dashboard_id dashboard-id
+ :position 0}
+ :model/DashboardTab {dashtab-id-2 :id} {:name "Tab 2"
+ :dashboard_id dashboard-id
+ :position 1}
+ DashboardCard {dashcard-id-1 :id} {:dashboard_id dashboard-id
+ :card_id card-id-1
+ :dashboard_tab_id dashtab-id-1}
+ DashboardCard {dashcard-id-2 :id} {:dashboard_id dashboard-id
+ :card_id card-id-2
+ :dashboard_tab_id dashtab-id-2}]
+ (f {:dashboard-id dashboard-id
+ :card-id-1 card-id-1
+ :card-id-2 card-id-1
+ :dashtab-id-1 dashtab-id-1
+ :dashtab-id-2 dashtab-id-2
+ :dashcard-id-1 dashcard-id-1
+ :dashcard-id-2 dashcard-id-2})))
+
+(defmacro ^:private with-simple-dashboard-with-tabs
+ "Create a simple dashboard with 2 tabs and 2 cards in each tab and run `body` with the dashboard and cards ids bound to"
+ [[bindings] & body]
+ `(do-with-simple-dashboard-with-tabs (fn [~bindings] ~@body)))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | /api/dashboard/* AUTHENTICATION Tests |
@@ -868,6 +900,7 @@
"The copy should have a new entity ID generated")
(finally
(t2/delete! Dashboard :id (u/the-id response))))))))
+
(testing "Deep copy: POST /api/dashboard/:id/copy"
(mt/dataset sample-dataset
(mt/with-temp* [Collection [source-coll {:name "Source collection"}]
@@ -1095,6 +1128,31 @@
(set (map :name cards-in-coll)))
"Cards should have \"-- Duplicate\" appended"))))))))
+(deftest copy-dashboard-with-tab-test
+ (testing "POST /api/dashboard/:id/copy"
+ (testing "for a dashboard that has tabs"
+ (with-simple-dashboard-with-tabs [{:keys [dashboard-id]}]
+ (let [new-dash-id (:id (mt/user-http-request :rasta :post 200
+ (format "dashboard/%d/copy" dashboard-id)
+ {:name "New dashboard"
+ :description "A new description"}))
+ original-tabs (t2/select [:model/DashboardTab :id :position :name]
+ :dashboard_id dashboard-id
+ {:order-by [[:position :asc]]})
+ new-tabs (t2/select [:model/DashboardTab :id :position :name]
+ :dashboard_id new-dash-id
+ {:order-by [[:position :asc]]})
+ new->old-tab-id (zipmap (map :id new-tabs) (map :id original-tabs))]
+ (testing "Cards are located correctly between tabs"
+ (is (= (map #(select-keys % [:name :display :dashboard_tab_id]) (dashboard/ordered-cards dashboard-id))
+ (map #(select-keys % [:name :display :dashboard_tab_id])
+ (for [card (dashboard/ordered-cards new-dash-id)]
+ (assoc card :dashboard_tab_id (new->old-tab-id (:dashboard_tab_id card))))))))
+
+ (testing "new tabs should have the same name and position"
+ (is (= (map #(dissoc % :id) original-tabs)
+ (map #(dissoc % :id) new-tabs)))))))))
+
(def ^:dynamic ^:private
^{:doc "Set of ids that will report [[mi/can-write]] as true."}
*readable-card-ids* #{})
@@ -1140,14 +1198,24 @@
(deftest update-cards-for-copy-test
(testing "When copy style is shallow returns original ordered-cards"
-
(let [ordered-cards [{:card_id 1 :card {:id 1} :series [{:id 2}]}
{:card_id 3 :card {:id 3}}]]
(is (= ordered-cards
(api.dashboard/update-cards-for-copy 1
ordered-cards
false
- nil)))))
+ nil
+ 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 1
+ [{:card_id 1 :card {:id 1} :dashboard_tab_id 1}
+ {:card_id 3 :card {:id 3} :dashboard_tab_id 2}]
+ false
+ nil
+ {1 10
+ 2 20})))))
(testing "When copy style is deep"
(let [ordered-cards [{:card_id 1 :card {:id 1} :series [{:id 2} {:id 3}]}]]
(testing "Can omit series cards"
@@ -1156,7 +1224,8 @@
ordered-cards
true
{1 {:id 5}
- 2 {:id 6}})))))
+ 2 {:id 6}}
+ nil)))))
(testing "Can omit whole card with series if not copied"
(let [ordered-cards [{:card_id 1 :card {} :series [{:id 2} {:id 3}]}
{:card_id 4 :card {} :series [{:id 5} {:id 6}]}]]
@@ -1169,7 +1238,8 @@
3 {:id 9}
;; not copying id 4 which is the base of the following two
5 {:id 10}
- 6 {:id 11}})))))
+ 6 {:id 11}}
+ nil)))))
(testing "Updates parameter mappings to new card ids"
(let [ordered-cards [{:card_id 1
:card {:id 1}
@@ -1186,7 +1256,8 @@
(api.dashboard/update-cards-for-copy 1
ordered-cards
true
- {1 {:id 2}})))))
+ {1 {:id 2}}
+ nil)))))
(testing "Throws error if no new card-id information for deep copy"
(let [user-id 44
dashboard-id 55
@@ -1195,6 +1266,7 @@
(api.dashboard/update-cards-for-copy dashboard-id
[{:card_id 1 :card {:id 1}}]
true
+ nil
nil))
(is false "Should have thrown with deep-copy true and no new card id info")
(catch Exception e (ex-data e)))]
@@ -1317,40 +1389,6 @@
(format "dashboard/%d/cards" dashboard-id)
{:cards [new-dashcard-info]
:ordered_tabs []}))}))))))
-
-(defn- do-with-simple-dashboard-with-tabs
- [f]
- (t2.with-temp/with-temp
- [Dashboard {dashboard-id :id} {}
-
- Card {card-id-1 :id} {}
-
- Card {card-id-2 :id} {}
- :model/DashboardTab {dashtab-id-1 :id} {:name "Tab 1"
- :dashboard_id dashboard-id
- :position 0}
- :model/DashboardTab {dashtab-id-2 :id} {:name "Tab 2"
- :dashboard_id dashboard-id
- :position 1}
- DashboardCard {dashcard-id-1 :id} {:dashboard_id dashboard-id
- :card_id card-id-1
- :dashboard_tab_id dashtab-id-1}
- DashboardCard {dashcard-id-2 :id} {:dashboard_id dashboard-id
- :card_id card-id-2
- :dashboard_tab_id dashtab-id-2}]
- (f {:dashboard-id dashboard-id
- :card-id-1 card-id-1
- :card-id-2 card-id-1
- :dashtab-id-1 dashtab-id-1
- :dashtab-id-2 dashtab-id-2
- :dashcard-id-1 dashcard-id-1
- :dashcard-id-2 dashcard-id-2})))
-
-(defmacro ^:private with-simple-dashboard-with-tabs
- "Create a simple dashboard with 2 tabs and 2 cards in each tab and run `body` with the dashboard and cards ids bound to"
- [[bindings] & body]
- `(do-with-simple-dashboard-with-tabs (fn [~bindings] ~@body)))
-
(deftest e2e-update-cards-and-tabs-test
(testing "PUT /api/dashboard/:id/cards with create/update/delete in a single req"
(t2.with-temp/with-temp