diff --git a/src/metabase/api/collection.clj b/src/metabase/api/collection.clj index 554d3bd125fa76fa4f084ca56f0511cb36be548f..d3b19ed2335c6e20d658212809a65ac7df288855 100644 --- a/src/metabase/api/collection.clj +++ b/src/metabase/api/collection.clj @@ -4,7 +4,8 @@ `:snippet` namespace, (called 'Snippet folders' in the UI). These namespaces are completely independent hierarchies. To use these endpoints for other Collections namespaces, you can pass the `?namespace=` parameter (e.g. `?namespace=snippet`)." - (:require [clojure.string :as str] + (:require [cheshire.core :as json] + [clojure.string :as str] [compojure.core :refer [GET POST PUT]] [honeysql.core :as hsql] [honeysql.helpers :as hh] @@ -205,7 +206,7 @@ (for [row rows] (dissoc row :description :display :authority_level :moderated_status :icon :personal_owner_id - :collection_preview))) + :collection_preview :dataset_query))) (defmethod collection-children-query :snippet [_ collection {:keys [archived?]}] @@ -229,17 +230,19 @@ (for [row rows] (dissoc row :description :display :collection_position :authority_level :moderated_status - :collection_preview))) + :collection_preview :dataset_query))) (defmethod post-process-collection-children :snippet [_ rows] (for [row rows] (dissoc row :description :collection_position :display :authority_level - :moderated_status :icon :personal_owner_id :collection_preview))) + :moderated_status :icon :personal_owner_id :collection_preview + :dataset_query))) (defn- card-query [dataset? collection {:keys [archived? pinned-state]}] (-> {:select [:c.id :c.name :c.description :c.entity_id :c.collection_position :c.display :c.collection_preview + :c.dataset_query [(hx/literal (if dataset? "dataset" "card")) :model] [:u.id :last_edit_user] [:u.email :last_edit_email] [:u.first_name :last_edit_first_name] [:u.last_name :last_edit_last_name] @@ -293,10 +296,16 @@ (not (zero? v)) v)) +(defn- has-required-parameters? [row] + (if-let [template-tags (-> row :dataset_query (json/parse-string keyword) :native :template-tags)] + (every? #(or (not (:required %)) (:default %)) (vals template-tags)) + true)) + (defn- post-process-card-row [row] (-> row - (dissoc :authority_level :icon :personal_owner_id) - (update :collection_preview bit->boolean))) + (dissoc :authority_level :icon :personal_owner_id :dataset_query) + (update :collection_preview bit->boolean) + (assoc :has_required_parameters (has-required-parameters? row)))) (defmethod post-process-collection-children :card [_ rows] @@ -327,7 +336,9 @@ (defmethod post-process-collection-children :dashboard [_ rows] - (map #(dissoc % :display :authority_level :moderated_status :icon :personal_owner_id :collection_preview) + (map #(dissoc % + :display :authority_level :moderated_status :icon :personal_owner_id :collection_preview + :dataset_query) rows)) (defmethod collection-children-query :collection @@ -360,7 +371,7 @@ (:personal_owner_id row) (assoc :name (collection/user->personal-collection-name (:personal_owner_id row) :user)) true (assoc :can_write (mi/can-write? Collection (:id row))) true (dissoc :collection_position :display :moderated_status :icon :personal_owner_id - :collection_preview)))) + :collection_preview :dataset_query)))) (s/defn ^:private coalesce-edit-info :- last-edit/MaybeAnnotated "Hoist all of the last edit information into a map under the key :last-edit-info. Considers this information present @@ -418,7 +429,7 @@ "All columns that need to be present for the union-all. Generated with the comment form below. Non-text columns that are optional (not id, but last_edit_user for example) must have a type so that the union-all can unify the nil with the correct column type." - [:id :name :description :entity_id :display [:collection_preview :boolean] + [:id :name :description :entity_id :display [:collection_preview :boolean] :dataset_query :model :collection_position :authority_level [:personal_owner_id :integer] :last_edit_email :last_edit_first_name :last_edit_last_name :moderated_status :icon [:last_edit_user :integer] [:last_edit_timestamp :timestamp]]) diff --git a/test/metabase/api/collection_test.clj b/test/metabase/api/collection_test.clj index 161ad4a9905ceed85eab38c510e58d0f45352ca3..ed452f8b14ffc25c5590ccf7e927c153ed4d6c49 100644 --- a/test/metabase/api/collection_test.clj +++ b/test/metabase/api/collection_test.clj @@ -418,15 +418,16 @@ :moderator_id user-id :most_recent true}]] (is (= (mt/obj->json->obj - [{:id card-id - :name (:name card) - :collection_position nil - :collection_preview true - :display "table" - :description nil - :entity_id (:entity_id card) - :moderated_status "verified" - :model "card"}]) + [{:id card-id + :name (:name card) + :collection_position nil + :collection_preview true + :display "table" + :description nil + :entity_id (:entity_id card) + :moderated_status "verified" + :model "card" + :has_required_parameters true}]) (mt/obj->json->obj (:data (mt/user-http-request :crowberto :get 200 (str "collection/" (u/the-id collection) "/items")))))))) @@ -466,11 +467,12 @@ (mt/with-temp Collection [collection {:name "Debt Collection"}] (perms/grant-collection-read-permissions! (perms-group/all-users) collection) (with-some-children-of-collection collection - (is (= (map default-item [{:name "Acme Products", :model "pulse", :entity_id true} - {:name "Birthday Card", :description nil, :model "card", - :collection_preview false, :display "table", :entity_id true} - {:name "Dine & Dashboard", :description nil, :model "dashboard", :entity_id true} - {:name "Electro-Magnetic Pulse", :model "pulse", :entity_id true}]) + (is (= (-> (mapv default-item [{:name "Acme Products", :model "pulse", :entity_id true} + {:name "Birthday Card", :description nil, :model "card", + :collection_preview false, :display "table", :entity_id true} + {:name "Dine & Dashboard", :description nil, :model "dashboard", :entity_id true} + {:name "Electro-Magnetic Pulse", :model "pulse", :entity_id true}]) + (assoc-in [1 :has_required_parameters] true)) (mt/boolean-ids-and-timestamps (:data (mt/user-http-request :rasta :get 200 (str "collection/" (u/the-id collection) "/items")))))))) @@ -480,17 +482,18 @@ (with-some-children-of-collection collection (is (= () (mt/boolean-ids-and-timestamps - (:data (mt/user-http-request :rasta :get 200 (str "collection/" (u/the-id collection) "/items?models=no_models")))))) + (:data (mt/user-http-request :rasta :get 200 (str "collection/" (u/the-id collection) "/items?models=no_models")))))) (is (= [(default-item {:name "Dine & Dashboard", :description nil, :model "dashboard", :entity_id true})] (mt/boolean-ids-and-timestamps (:data (mt/user-http-request :rasta :get 200 (str "collection/" (u/the-id collection) "/items?models=dashboard")))))) - (is (= [(default-item {:name "Birthday Card", :description nil, :model "card", - :collection_preview false, :display "table", :entity_id true}) + (is (= [(-> {:name "Birthday Card", :description nil, :model "card", + :collection_preview false, :display "table", :entity_id true} + default-item + (assoc :has_required_parameters true)) (default-item {:name "Dine & Dashboard", :description nil, :model "dashboard", :entity_id true})] (mt/boolean-ids-and-timestamps (:data (mt/user-http-request :rasta :get 200 (str "collection/" (u/the-id collection) "/items?models=dashboard&models=card")))))))))) - (testing "Let's make sure the `archived` option works." (mt/with-temp Collection [collection {:name "Art Collection"}] (perms/grant-collection-read-permissions! (perms-group/all-users) collection) @@ -934,8 +937,10 @@ (deftest fetch-root-items-collection-test (testing "GET /api/collection/root/items" (testing "Make sure you can see everything for Users that can see everything" - (is (= [(default-item {:name "Birthday Card", :description nil, :model "card", - :collection_preview false, :display "table"}) + (is (= [(-> {:name "Birthday Card", :description nil, :model "card", + :collection_preview false, :display "table"} + default-item + (assoc :has_required_parameters true)) (collection-item "Crowberto Corv's Personal Collection") (default-item {:name "Dine & Dashboard", :description nil, :model "dashboard"}) (default-item {:name "Electro-Magnetic Pulse", :model "pulse"})] @@ -978,8 +983,10 @@ (mt/with-temp* [PermissionsGroup [group] PermissionsGroupMembership [_ {:user_id (mt/user->id :rasta), :group_id (u/the-id group)}]] (perms/grant-permissions! group (perms/collection-read-path {:metabase.models.collection.root/is-root? true})) - (is (= [(default-item {:name "Birthday Card", :description nil, :model "card", - :collection_preview false, :display "table"}) + (is (= [(-> {:name "Birthday Card", :description nil, :model "card", + :collection_preview false, :display "table"} + default-item + (assoc :has_required_parameters true)) (default-item {:name "Dine & Dashboard", :description nil, :model "dashboard"}) (default-item {:name "Electro-Magnetic Pulse", :model "pulse"}) (collection-item "Rasta Toucan's Personal Collection")] @@ -1040,18 +1047,67 @@ (testing "Can we look for `archived` stuff with this endpoint?" (mt/with-temp Card [card {:name "Business Card", :archived true}] - (is (= [{:name "Business Card" - :description nil - :collection_position nil - :collection_preview true - :display "table" - :moderated_status nil - :entity_id (:entity_id card) - :model "card"}] + (is (= [{:name "Business Card" + :description nil + :collection_position nil + :collection_preview true + :display "table" + :moderated_status nil + :entity_id (:entity_id card) + :model "card" + :has_required_parameters true}] (for [item (:data (mt/user-http-request :crowberto :get 200 "collection/root/items?archived=true"))] + (dissoc item :id))))))) + + (testing "has_required_parameters of a card" + (testing "can be false" + (mt/with-temp Card [card {:name "Business Card" + :dataset_query {:native {:template-tags {:param0 {:required true, :default 0} + :param1 {:required true} + :param2 {:required false}}}}}] + (is (= (let [collection (collection/user->personal-collection (mt/user->id :crowberto))] + [{:name "Business Card" + :description nil + :collection_position nil + :collection_preview true + :display "table" + :moderated_status nil + :entity_id (:entity_id card) + :model "card" + :has_required_parameters false} + {:name "Crowberto Corv's Personal Collection" + :description nil + :model "collection" + :authority_level nil + :entity_id (:entity_id collection) + :can_write true}]) + (for [item (:data (mt/user-http-request :crowberto :get 200 "collection/root/items"))] + (dissoc item :id)))))) + + (testing "is true if all required parameters have defaults" + (mt/with-temp Card [card {:name "Business Card" + :dataset_query {:native {:template-tags {:param0 {:required true, :default 0} + :param1 {:required true, :default 1} + :param2 {:required false}}}}}] + (is (= (let [collection (collection/user->personal-collection (mt/user->id :crowberto))] + [{:name "Business Card" + :description nil + :collection_position nil + :collection_preview true + :display "table" + :moderated_status nil + :entity_id (:entity_id card) + :model "card" + :has_required_parameters true} + {:name "Crowberto Corv's Personal Collection" + :description nil + :model "collection" + :authority_level nil + :entity_id (:entity_id collection) + :can_write true}]) + (for [item (:data (mt/user-http-request :crowberto :get 200 "collection/root/items"))] (dissoc item :id))))))))) - ;;; ----------------------------------- Effective Children, Ancestors, & Location ------------------------------------ (defn- api-get-root-collection-ancestors