diff --git a/src/metabase/api/pulse.clj b/src/metabase/api/pulse.clj index 81e0aa29e76836c9931c77d587c52ed4d65088e1..a59f438eaf7e4111bf79212569de01b0ea661c64 100644 --- a/src/metabase/api/pulse.clj +++ b/src/metabase/api/pulse.clj @@ -339,11 +339,12 @@ collection_id [:maybe ms/PositiveInt] collection_position [:maybe ms/PositiveInt] dashboard_id [:maybe ms/PositiveInt]} - ;; Check permissions on cards that exist. Placeholders don't matter. + ;; Check permissions on cards that exist. Placeholders and iframes don't matter. (check-card-read-permissions (remove (fn [{:keys [id display]}] (and (nil? id) - (= "placeholder" display))) cards)) + (or (= "placeholder" display) + (= "iframe" display)))) cards)) ;; make sure any email addresses that are specified are allowed before sending the test Pulse. (doseq [channel channels] (pulse-channel/validate-email-domains channel)) diff --git a/src/metabase/models/recent_views.clj b/src/metabase/models/recent_views.clj index f127040b27f86ba8fa7d67fa0888a805c145e9f1..d64c829c299007f2b53626fbf764253e43c10780 100644 --- a/src/metabase/models/recent_views.clj +++ b/src/metabase/models/recent_views.clj @@ -419,7 +419,7 @@ (when (and (not= "hidden" (:visibility_type table)) (:database-name table) (:active table) - (mi/can-read? table)) + (mi/can-read? :model/Table model_id)) {:id model_id :name (:name table) :description (:description table) diff --git a/src/metabase/pulse.clj b/src/metabase/pulse.clj index 2f8cd8cf04ae35fe7f19fb723b3c72598e9eccde..5e141d8869553eb010f3ed79349c4e6a43dd2d9e 100644 --- a/src/metabase/pulse.clj +++ b/src/metabase/pulse.clj @@ -112,6 +112,10 @@ (let [parameters (merge-default-values (pulse-params/parameters pulse dashboard))] (pu/execute-dashboard-subscription-card dashcard parameters)) + ;; iframes + (virtual-card-of-type? dashcard "iframe") + nil + ;; actions (virtual-card-of-type? dashcard "action") nil diff --git a/test/metabase/dashboard_subscription_test.clj b/test/metabase/dashboard_subscription_test.clj index 3b9cd22c49f96cd01028d35f82c100aeb6d20b12..ba0d533b76baa349dcf66315e55163d6a72e4185 100644 --- a/test/metabase/dashboard_subscription_test.clj +++ b/test/metabase/dashboard_subscription_test.clj @@ -768,6 +768,58 @@ {:text (format "### [https://metabase.com](https://metabase.com)")}] (@#'metabase.pulse/execute-dashboard {:creator_id (mt/user->id :lucky)} dashboard))))))))) +(deftest iframe-cards-are-skipped-test + (testing "iframe cards should be filtered out" + (t2.with-temp/with-temp + [Dashboard {dashboard-id :id + :as dashboard} {:name "Dashboard"} + DashboardCard _ {:dashboard_id dashboard-id + :visualization_settings {:text "Markdown"} + :row 1} + DashboardCard _ {:dashboard_id dashboard-id + :visualization_settings {:virtual_card {:display "link"} + :link {:url "https://metabase.com"}} + :row 2} + DashboardCard _ {:dashboard_id dashboard-id + :visualization_settings {:virtual_card {:display "iframe"}} + :row 3}] + (is (=? [{:text "Markdown"} + {:text "### [https://metabase.com](https://metabase.com)"}] + (@#'metabase.pulse/execute-dashboard {:creator_id (mt/user->id :rasta)} dashboard))))) + + (testing "Link cards are returned and info should be newly fetched" + (t2.with-temp/with-temp [Dashboard dashboard {:name "Test Dashboard"}] + (with-link-card-fixture-for-dashboard dashboard [{:keys [collection-owner-id + collection-id + database-id + table-id + card-id + model-id + dashboard-id]}] + (let [site-url (public-settings/site-url)] + (testing "should returns all link cards and name are newly fetched" + (doseq [[model id] [[Card card-id] + [Table table-id] + [Database database-id] + [Dashboard dashboard-id] + [Collection collection-id] + [Card model-id]]] + (t2/update! model id {:name (format "New %s name" (name model))})) + (is (=? [{:text (format "### [New Collection name](%s/collection/%d)\nLinked collection desc" site-url collection-id)} + {:text (format "### [New Database name](%s/browse/%d)\nLinked database desc" site-url database-id)} + {:text (format "### [Linked table dname](%s/question?db=%d&table=%d)\nLinked table desc" site-url database-id table-id)} + {:text (format "### [New Dashboard name](%s/dashboard/%d)\nLinked Dashboard desc" site-url dashboard-id)} + {:text (format "### [New Card name](%s/question/%d)\nLinked card desc" site-url card-id)} + {:text (format "### [New Card name](%s/question/%d)\nLinked model desc" site-url model-id)} + {:text (format "### [https://metabase.com](https://metabase.com)")}] + (@#'metabase.pulse/execute-dashboard {:creator_id collection-owner-id} dashboard)))) + + (testing "it should filter out models that current users does not have permission to read" + (is (=? [{:text (format "### [New Database name](%s/browse/%d)\nLinked database desc" site-url database-id)} + {:text (format "### [Linked table dname](%s/question?db=%d&table=%d)\nLinked table desc" site-url database-id table-id)} + {:text (format "### [https://metabase.com](https://metabase.com)")}] + (@#'metabase.pulse/execute-dashboard {:creator_id (mt/user->id :lucky)} dashboard))))))))) + (deftest execute-dashboard-with-tabs-test (t2.with-temp/with-temp [Dashboard {dashboard-id :id diff --git a/test/metabase/models/recent_views_test.clj b/test/metabase/models/recent_views_test.clj index bb270a4ea9ddae1c4c4b925da174102670240609..6ea0d2f729ad0e2784fed377ebd7c37ae170c123 100644 --- a/test/metabase/models/recent_views_test.clj +++ b/test/metabase/models/recent_views_test.clj @@ -180,7 +180,16 @@ (is (= expected (mapv fixup (mt/with-test-user :rasta - (recent-views (mt/user->id :rasta)))))))))) + (recent-views (mt/user->id :rasta))))))))) + (testing "non admins can see tables in recents (#47420)" + (mt/dataset test-data + (let [products-id (mt/id :products)] + (assert (-> (mt/fetch-user :rasta) :is_superuser not) "User is a super user") + (recent-views/update-users-recent-views! (mt/user->id :rasta) :model/Table products-id :view) + (let [views (mt/with-test-user :rasta + (recent-views (mt/user->id :rasta)))] + (is (contains? (into #{} (map (juxt :id :display_name :model)) views) + [products-id "Products" :table]))))))) (deftest update-users-recent-views!-duplicates-test (testing "`update-users-recent-views!` prunes duplicates of a certain model.`"