Skip to content
Snippets Groups Projects
Unverified Commit 2d36fd21 authored by metamben's avatar metamben Committed by GitHub
Browse files

Add has_required_parameters flag for cards in collection queries (#23486)

parent 484f345b
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/api/collection.clj
+ 20
− 9
View file @ 2d36fd21
......@@ -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]])
......
test/metabase/api/collection_test.clj
+ 87
− 31
View file @ 2d36fd21
......@@ -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
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment