diff --git a/src/metabase/api/embed.clj b/src/metabase/api/embed.clj index d0906e17e474e3584d7aafa27ddd6f5cf0a3a562..005cd43581b445d258d4ddb67eaccba8ec4facf5 100644 --- a/src/metabase/api/embed.clj +++ b/src/metabase/api/embed.clj @@ -29,11 +29,39 @@ [metabase.query-processor.pivot :as qp.pivot] [metabase.util :as u] [metabase.util.embed :as embed] + [metabase.util.malli :as mu] [metabase.util.malli.schema :as ms] [toucan2.core :as t2])) (set! *warn-on-reflection* true) +(def ^:private ResourceId [:or ms/PositiveInt ms/NanoIdString]) +(def ^:private Token [:map + [:resource [:map + [:question {:optional true} ResourceId] + [:dashboard {:optional true} ResourceId]]] + [:params :any]]) + +(defn- conditional-update-in + "If there's a value at `path`, apply `f`, otherwise return `m`." + [m path f] + (if-let [value (get-in m path)] + (assoc-in m path (f value)) + m)) + +(mu/defn translate-token-ids :- Token + "Translate `entity_id` keys to `card_id` and `dashboard_id` respectively." + [unsigned :- Token] + (-> unsigned + (conditional-update-in [:resource :question] #(api.embed.common/->id :model/Card %)) + (conditional-update-in [:resource :dashboard] #(api.embed.common/->id :model/Dashboard %)))) + +(defn unsign-and-translate-ids + "Unsign a JWT and translate `entity_id` keys to `card_id` and `dashboard_id` respectively. If they are already + sequential ids, they are left as is." + [message] + (translate-token-ids (embed/unsign message))) + ;;; ------------------------------------------- /api/embed/card endpoints -------------------------------------------- (api/defendpoint GET "/card/:token" @@ -43,7 +71,7 @@ {:resource {:question <card-id>}}" [token] - (let [unsigned (embed/unsign token)] + (let [unsigned (unsign-and-translate-ids token)] (api.embed.common/check-embedding-enabled-for-card (embed/get-in-unsigned-token-or-throw unsigned [:resource :question])) (u/prog1 (api.embed.common/card-for-unsigned-token unsigned, :constraints [:enable_embedding true]) (events/publish-event! :event/card-read {:object-id (:id <>), :user-id api/*current-user-id*, :context :question})))) @@ -75,7 +103,7 @@ {:resource {:question <card-id>} :params <parameters>}" [token & query-params] - (run-query-for-unsigned-token-async (embed/unsign token) :api (api.embed.common/parse-query-params query-params))) + (run-query-for-unsigned-token-async (unsign-and-translate-ids token) :api (api.embed.common/parse-query-params query-params))) (api/defendpoint GET ["/card/:token/query/:export-format", :export-format api.dataset/export-format-regex] "Like `GET /api/embed/card/query`, but returns the results as a file in the specified format." @@ -83,7 +111,7 @@ {export-format (into [:enum] api.dataset/export-formats) format_rows [:maybe :boolean]} (run-query-for-unsigned-token-async - (embed/unsign token) + (unsign-and-translate-ids token) export-format (api.embed.common/parse-query-params (dissoc (m/map-keys keyword query-params) :format_rows)) :constraints nil @@ -100,7 +128,7 @@ {:resource {:dashboard <dashboard-id>}}" [token] - (let [unsigned (embed/unsign token)] + (let [unsigned (unsign-and-translate-ids token)] (api.embed.common/check-embedding-enabled-for-dashboard (embed/get-in-unsigned-token-or-throw unsigned [:resource :dashboard])) (u/prog1 (api.embed.common/dashboard-for-unsigned-token unsigned, :constraints [:enable_embedding true]) (events/publish-event! :event/dashboard-read {:object-id (:id <>), :user-id api/*current-user-id*})))) @@ -109,7 +137,7 @@ "Fetch the results of running a Card belonging to a Dashboard using a JSON Web Token signed with the `embedding-secret-key`. - Token should have the following format: + [[Token]] should have the following format: {:resource {:dashboard <dashboard-id>} :params <parameters>} @@ -121,7 +149,7 @@ & {:keys [constraints qp middleware] :or {constraints (qp.constraints/default-query-constraints) qp qp.card/process-query-for-card-default-qp}}] - (let [unsigned-token (embed/unsign token) + (let [unsigned-token (unsign-and-translate-ids token) dashboard-id (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :dashboard])] (api.embed.common/check-embedding-enabled-for-dashboard dashboard-id) (api.embed.common/process-query-for-dashcard @@ -156,7 +184,7 @@ "Fetch FieldValues for a Field that is referenced by an embedded Card." [token field-id] {field-id ms/PositiveInt} - (let [unsigned-token (embed/unsign token) + (let [unsigned-token (unsign-and-translate-ids token) card-id (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :question])] (api.embed.common/check-embedding-enabled-for-card card-id) (api.public/card-and-field-id->values card-id field-id))) @@ -165,7 +193,7 @@ "Fetch FieldValues for a Field that is used as a param in an embedded Dashboard." [token field-id] {field-id ms/PositiveInt} - (let [unsigned-token (embed/unsign token) + (let [unsigned-token (unsign-and-translate-ids token) dashboard-id (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :dashboard])] (api.embed.common/check-embedding-enabled-for-dashboard dashboard-id) (api.public/dashboard-and-field-id->values dashboard-id field-id))) @@ -179,7 +207,7 @@ search-field-id ms/PositiveInt value ms/NonBlankString limit [:maybe ms/PositiveInt]} - (let [unsigned-token (embed/unsign token) + (let [unsigned-token (unsign-and-translate-ids token) card-id (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :question])] (api.embed.common/check-embedding-enabled-for-card card-id) (api.public/search-card-fields card-id field-id search-field-id value (when limit (Integer/parseInt limit))))) @@ -191,7 +219,7 @@ search-field-id ms/PositiveInt value ms/NonBlankString limit [:maybe ms/PositiveInt]} - (let [unsigned-token (embed/unsign token) + (let [unsigned-token (unsign-and-translate-ids token) dashboard-id (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :dashboard])] (api.embed.common/check-embedding-enabled-for-dashboard dashboard-id) (api.public/search-dashboard-fields dashboard-id field-id search-field-id value (when limit @@ -206,7 +234,7 @@ {field-id ms/PositiveInt remapped-id ms/PositiveInt value ms/NonBlankString} - (let [unsigned-token (embed/unsign token) + (let [unsigned-token (unsign-and-translate-ids token) card-id (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :question])] (api.embed.common/check-embedding-enabled-for-card card-id) (api.public/card-field-remapped-values card-id field-id remapped-id value))) @@ -218,7 +246,7 @@ {field-id ms/PositiveInt remapped-id ms/PositiveInt value ms/NonBlankString} - (let [unsigned-token (embed/unsign token) + (let [unsigned-token (unsign-and-translate-ids token) dashboard-id (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :dashboard])] (api.embed.common/check-embedding-enabled-for-dashboard dashboard-id) (api.public/dashboard-field-remapped-values dashboard-id field-id remapped-id value))) @@ -265,7 +293,7 @@ (api/defendpoint GET "/card/:token/params/:param-key/values" "Embedded version of api.card filter values endpoint." [token param-key] - (let [unsigned (embed/unsign token) + (let [unsigned (unsign-and-translate-ids token) card-id (embed/get-in-unsigned-token-or-throw unsigned [:resource :question]) card (t2/select-one Card :id card-id)] (api.embed.common/check-embedding-enabled-for-card card-id) @@ -276,7 +304,7 @@ (api/defendpoint GET "/card/:token/params/:param-key/search/:prefix" "Embedded version of chain filter search endpoint." [token param-key prefix] - (let [unsigned (embed/unsign token) + (let [unsigned (unsign-and-translate-ids token) card-id (embed/get-in-unsigned-token-or-throw unsigned [:resource :question]) card (t2/select-one Card :id card-id)] (api.embed.common/check-embedding-enabled-for-card card-id) @@ -293,7 +321,7 @@ {:resource {:question <card-id>} :params <parameters>}" [token & query-params] - (run-query-for-unsigned-token-async (embed/unsign token) + (run-query-for-unsigned-token-async (unsign-and-translate-ids token) :api (api.embed.common/parse-query-params query-params) :qp qp.pivot/run-pivot-query)) diff --git a/src/metabase/api/embed/common.clj b/src/metabase/api/embed/common.clj index fb7db3c9b17730ab04aa98b22cec11476a749a45..87a7c756c51d6ec4da60e3c5e45c34803bae60d7 100644 --- a/src/metabase/api/embed/common.clj +++ b/src/metabase/api/embed/common.clj @@ -274,6 +274,99 @@ (assoc (select-keys param [:type :target :slug]) :value value)))) +;;; -------------------------------------- Entity ID transformation functions ------------------------------------------ + +(def ^:private api-models + "The models that we will service for entity-id transformations." + (->> (descendants :metabase/model) + (filter #(= (namespace %) "model")) + (filter (fn has-entity-id? + [model] (or ;; toucan1 models + (isa? model :metabase.models.interface/entity-id) + ;; toucan2 models + (isa? model :hook/entity-id)))) + (map keyword) + set)) + +(def ^:private api-name->model + "Map of model names used on the API to their corresponding model." + (->> api/model->db-model + (map (fn [[k v]] [(keyword k) (:db-model v)])) + (filter (fn [[_ v]] (contains? api-models v))) + (into {}))) + +(defn- ->model + "Takes a model keyword or an api-name and returns the corresponding model keyword." + [model-or-api-name] + (if (contains? api-models model-or-api-name) + model-or-api-name + (api-name->model model-or-api-name))) + +(def ^:private eid-api-models + "Sorted vec of api models that have an entity_id column" + (vec (sort (keys api-name->model)))) + +(def ^:private ApiModel (into [:enum] eid-api-models)) + +(def ^:private EntityId + "A Malli schema for an entity id, this is a little looser because it needs to be fast." + [:and {:description "entity_id"} + :string + [:fn {:error/fn (fn [{:keys [value]} _] + (str "\"" value "\" should be 21 characters long, but it is " (count value)))} + (fn eid-length-good? [eid] (= 21 (count eid)))]]) + +(def ^:private ModelToEntityIds + "A Malli schema for a map of model names to a sequence of entity ids." + (mc/schema [:map-of ApiModel [:sequential :string]])) + +(mu/defn- entity-ids->id-for-model + "Given a model and a sequence of entity ids on that model, return a pairs of entity-id, id." + [api-name eids] + (let [model (->model api-name) ;; This lookup is safe because we've already validated the api-names + eid->id (into {} (t2/select-fn->fn :entity_id :id [model :id :entity_id] :entity_id [:in eids]))] + (mapv (fn entity-id-info [entity-id] + [entity-id (if-let [id (get eid->id entity-id)] + {:id id :type api-name :status "ok"} + ;; handle errors + (if (mc/validate EntityId entity-id) + {:type api-name + :status "not-found"} + {:type api-name + :status "invalid-format" + :reason (me/humanize (mc/explain EntityId entity-id))}))]) + eids))) + +(defn model->entity-ids->ids + "Given a map of model names to a sequence of entity-ids for each, return a map from entity-id -> id." + [model-key->entity-ids] + (when-not (mc/validate ModelToEntityIds model-key->entity-ids) + (throw (ex-info "Invalid format." {:explanation (me/humanize + (me/with-spell-checking + (mc/explain ModelToEntityIds model-key->entity-ids))) + :allowed-models (sort (keys api-name->model)) + :status-code 400}))) + (into {} + (mapcat + (fn [[model eids]] (entity-ids->id-for-model model eids)) + model-key->entity-ids))) + +(mu/defn ->id :- :int + "Translates a single entity_id -> id. This reuses the batched version: [[model->entity-ids->ids]]. + Please use that if you have to do man lookups at once." + [pre-model id :- [:or :int :string]] + (if (string? id) + (let [model (->model pre-model) + [[_ {:keys [status] :as info}]] (entity-ids->id-for-model model [id])] + (if-not (= "ok" status) + (throw (ex-info "problem looking up id from entity_id" + {:pre-model pre-model + :model model + :id id + :status status})) + (:id info))) + id)) + ;;; ---------------------------- Card Fns used by both /api/embed and /api/preview_embed ----------------------------- (defn card-for-unsigned-token @@ -281,7 +374,8 @@ `public-card` function that fetches the Card." [unsigned-token & {:keys [embedding-params constraints]}] {:pre [((some-fn empty? sequential?) constraints) (even? (count constraints))]} - (let [card-id (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :question]) + (let [pre-card-id (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :question]) + card-id (->id :model/Card pre-card-id) token-params (embed/get-in-unsigned-token-or-throw unsigned-token [:params])] (-> (apply api.public/public-card :id card-id, constraints) api.public/combine-parameters-and-template-tags @@ -350,7 +444,8 @@ the `public-dashboard` function that fetches the Dashboard." [unsigned-token & {:keys [embedding-params constraints]}] {:pre [((some-fn empty? sequential?) constraints) (even? (count constraints))]} - (let [dashboard-id (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :dashboard]) + (let [pre-dashboard-id (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :dashboard]) + dashboard-id (->id :model/Dashboard pre-dashboard-id) embedding-params (or embedding-params (t2/select-one-fn :embedding_params :model/Dashboard, :id dashboard-id)) token-params (embed/get-in-unsigned-token-or-throw unsigned-token [:params])] @@ -443,7 +538,8 @@ [token searched-param-id prefix id-query-params & {:keys [preview] :or {preview false}}] (let [unsigned-token (embed/unsign token) - dashboard-id (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :dashboard]) + pre-dashboard-id (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :dashboard]) + dashboard-id (->id :model/Dashboard pre-dashboard-id) _ (when-not preview (check-embedding-enabled-for-dashboard dashboard-id)) slug-token-params (embed/get-in-unsigned-token-or-throw unsigned-token [:params]) {parameters :parameters @@ -490,67 +586,3 @@ e)] (log/errorf e "Chain filter error\n%s" (u/pprint-to-str (u/all-ex-data e))) (throw e)))))) - -;;; -------------------------------------- Entity ID transformation functions ------------------------------------------ - -(def ^:private api-models - "The models that we will service for entity-id transformations." - (->> (descendants :metabase/model) - (filter #(= (namespace %) "model")) - (filter (fn has-entity-id? - [model] (or ;; toucan1 models - (isa? model :metabase.models.interface/entity-id) - ;; toucan2 models - (isa? model :hook/entity-id)))) - (map keyword) - set)) - -(def ^:private api-name->model - "Map of model names used on the API to their corresponding model." - (->> api/model->db-model - (map (fn [[k v]] [(keyword k) (:db-model v)])) - (filter (fn [[_ v]] (contains? api-models v))) - (into {}))) - -(def ^:private eid-api-models - "Sorted vec of api models that have an entity_id column" - (vec (sort (keys api-name->model)))) - -(def ^:private ApiModel (into [:enum] eid-api-models)) - -(def ^:private EntityId - "A Malli schema for an entity id, this is a little looser because it needs to be fast." - [:and {:description "entity_id"} - :string - [:fn {:error/fn (fn [{:keys [value]} _] - (str "\"" value "\" should be 21 characters long, but it is " (count value)))} - (fn eid-length-good? [eid] (= 21 (count eid)))]]) - -(def ^:private ModelToEntityIds - "A Malli schema for a map of model names to a sequence of entity ids." - (mc/schema [:map-of ApiModel [:sequential EntityId]])) - -(mu/defn- entity-ids->id-for-model - "Given a model and a sequence of entity ids on that model, return a pairs of entity-id, id." - [api-name eids] - (let [model (api-name->model api-name) ;; This lookup is safe because we've already validated the api-names - eid->id (into {} (t2/select-fn->fn :entity_id :id [model :id :entity_id] :entity_id [:in eids]))] - (mapv (fn [entity-id] - [entity-id (if-let [id (get eid->id entity-id)] - {:id id :type api-name} - {:type api-name :status "not-found"})]) - eids))) - -(defn model->entity-ids->ids - "Given a map of model names to a sequence of entity-ids for each, return a map from entity-id -> id." - [model-key->entity-ids] - (when-not (mc/validate ModelToEntityIds model-key->entity-ids) - (throw (ex-info "Invalid format." {:explanation (me/humanize - (me/with-spell-checking - (mc/explain ModelToEntityIds model-key->entity-ids))) - :allowed-models (sort (keys api-name->model)) - :status-code 400}))) - (into {} - (mapcat - (fn [[model eids]] (entity-ids->id-for-model model eids)) - model-key->entity-ids))) diff --git a/test/metabase/api/embed_test.clj b/test/metabase/api/embed_test.clj index e84b87b60aef4c7932f34cc6d6b11414d8264ed9..053918296572277b7f54ce091fc345cbc188be70 100644 --- a/test/metabase/api/embed_test.clj +++ b/test/metabase/api/embed_test.clj @@ -47,15 +47,21 @@ (defmacro with-new-secret-key! {:style/indent 0} [& body] `(do-with-new-secret-key! (fn [] ~@body))) -(defn card-token [card-or-id & [additional-token-params]] - (sign (merge {:resource {:question (u/the-id card-or-id)} +(defn- the-id-or-entity-id + "u/the-id doesn't work on entity-ids, so we should just pass them through." + [id-or-entity-id] + (try (u/the-id id-or-entity-id) + (catch Exception _ id-or-entity-id))) + +(defn card-token [card-or-id & [additional-token-keys]] + (sign (merge {:resource {:question (the-id-or-entity-id card-or-id)} :params {}} - additional-token-params))) + additional-token-keys))) -(defn dash-token [dash-or-id & [additional-token-params]] - (sign (merge {:resource {:dashboard (u/the-id dash-or-id)} +(defn dash-token [dash-or-id & [additional-token-keys]] + (sign (merge {:resource {:dashboard (the-id-or-entity-id dash-or-id)} :params {}} - additional-token-params))) + additional-token-keys))) (defn do-with-temp-card [m f] (let [m (merge (when-not (:dataset_query m) @@ -177,8 +183,8 @@ (with-embedding-enabled-and-new-secret-key! (let [card-url (str "embed/card/" (sign {:resource {:question "8"} :params {}}))] - (is (= "Card id should be a positive integer." - (client/client :get 400 card-url)))))) + (is #(re-matches #"Invalid input:.+value must be an integer greater than zero.+got.+8" + (client/client :get 400 card-url)))))) (deftest check-that-the-endpoint-doesn-t-work-if-embedding-isn-t-enabled (mt/with-temporary-setting-values [enable-embedding false] @@ -277,10 +283,10 @@ (defn card-query-url "Generate a query URL for an embedded card" - [card response-format-route-suffix & [additional-token-params]] + [card-or-id response-format-route-suffix & [additional-token-keys]] {:pre [(#{"" "/json" "/csv" "/xlsx"} response-format-route-suffix)]} (str "embed/card/" - (card-token card additional-token-params) + (card-token card-or-id additional-token-keys) "/query" response-format-route-suffix)) @@ -311,7 +317,9 @@ (with-new-secret-key! (with-temp-card [card] (is (= "Embedding is not enabled." - (client/real-client :get 400 (card-query-url card response-format)))))))))))) + (client/real-client :get 400 (card-query-url card response-format)))) + (is (= "Embedding is not enabled." + (client/real-client :get 400 (card-query-url (:entity_id card) response-format {})))))))))))) (deftest card-query-test-2 (testing "GET /api/embed/card/:token/query and GET /api/embed/card/:token/query/:export-format" @@ -325,6 +333,11 @@ (test-query-results response-format (client/real-client :get expected-status (card-query-url card response-format) + {:request-options request-options})) + #_{:clj-kondo/ignore [:deprecated-var]} + (test-query-results + response-format + (client/real-client :get expected-status (card-query-url (:entity_id card) response-format) {:request-options request-options})))))))))) (deftest card-query-test-3 @@ -341,7 +354,11 @@ (is (= {:status "failed" :error "An error occurred while running the query." :error_type "invalid-query"} - (client/real-client :get expected-status (card-query-url card response-format)))))))))))) + (client/real-client :get expected-status (card-query-url card response-format)))) + (is (= {:status "failed" + :error "An error occurred while running the query." + :error_type "invalid-query"} + (client/real-client :get expected-status (card-query-url (:entity_id card) response-format)))))))))))) (deftest card-query-test-4 (testing "GET /api/embed/card/:token/query and GET /api/embed/card/:token/query/:export-format" @@ -351,7 +368,11 @@ (testing "check that if embedding *is* enabled globally but not for the Card the request fails" (with-temp-card [card] (is (= "Embedding is not enabled for this object." - (client/real-client :get 400 (card-query-url card response-format))))))))))) + (client/real-client :get 400 (card-query-url card response-format) {}))))) + (testing "check that if embedding *is* enabled globally but not for the Card the request fails with entity ids" + (with-temp-card [card] + (is (= "Embedding is not enabled for this object." + (client/real-client :get 400 (card-query-url (:entity_id card) response-format) {})))))))))) (deftest card-query-test-5 (testing "GET /api/embed/card/:token/query and GET /api/embed/card/:token/query/:export-format" @@ -362,7 +383,9 @@ "signed with the wrong key") (with-temp-card [card {:enable_embedding true}] (is (= "Message seems corrupt or manipulated" - (client/real-client :get 400 (with-new-secret-key! (card-query-url card response-format)))))))))))) + (client/real-client :get 400 (with-new-secret-key! (card-query-url card response-format))))) + (is (= "Message seems corrupt or manipulated" + (client/real-client :get 400 (with-new-secret-key! (card-query-url (:entity_id card) response-format)))))))))))) (deftest download-formatted-without-constraints-test (testing (str "Downloading CSV/JSON/XLSX results shouldn't be subject to the default query constraints -- even if " @@ -377,7 +400,10 @@ :userland-query? true})}] (let [results (client/client :get 200 (card-query-url card "/csv"))] (is (= 101 - (count (csv/read-csv results)))))))))) + (count (csv/read-csv results))))) + (let [entity-id-results (client/client :get 200 (card-query-url (:entity_id card) "/csv"))] + (is (= 101 + (count (csv/read-csv entity-id-results)))))))))) (deftest card-locked-params-test (mt/test-helpers-set-global-values! @@ -524,22 +550,27 @@ ;;; ---------------------------------------- GET /api/embed/dashboard/:token ----------------------------------------- -(defn dashboard-url [dashboard & [additional-token-params]] - (str "embed/dashboard/" (dash-token dashboard additional-token-params))) +(defn dashboard-url [dashboard & [additional-token-keys entity-id]] + (str "embed/dashboard/" (dash-token dashboard additional-token-keys entity-id))) (deftest it-should-be-possible-to-call-this-endpoint-successfully (with-embedding-enabled-and-new-secret-key! (t2.with-temp/with-temp [Dashboard dash {:enable_embedding true}] (is (= successful-dashboard-info (dissoc-id-and-name - (client/client :get 200 (dashboard-url dash)))))))) + (client/client :get 200 (dashboard-url dash))))) + (is (= successful-dashboard-info + (dissoc-id-and-name + (client/client :get 200 (dashboard-url (:entity_id dash) dash)))))))) (deftest bad-dashboard-id-fails (with-embedding-enabled-and-new-secret-key! (let [dashboard-url (str "embed/dashboard/" (sign {:resource {:dashboard "8"} :params {}}))] - (is (= "Dashboard id should be a positive integer." - (client/client :get 400 dashboard-url)))))) + (is (contains? + #{"Dashboard id should be a positive integer." + "Invalid input: {:resource {:dashboard [\"value must be an integer greater than zero., got: \\\"8\\\"\" \"String must be a valid 21-character NanoID string., got: \\\"8\\\"\"]}}"} + (client/client :get 400 dashboard-url)))))) (deftest we-should-fail-when-attempting-to-use-an-expired-token-2 (with-embedding-enabled-and-new-secret-key! @@ -578,7 +609,9 @@ {:id "_c", :slug "c", :name "c", :type "date"} {:id "_d", :slug "d", :name "d", :type "date"}]}] (is (=? [{:id "_d", :slug "d", :name "d", :type "date"}] - (:parameters (client/client :get 200 (dashboard-url dash {:params {:c 100}}))))))))) + (:parameters (client/client :get 200 (dashboard-url dash {:params {:c 100}}))))) + (is (=? [{:id "_d", :slug "d", :name "d", :type "date"}] + (:parameters (client/client :get 200 (dashboard-url dash {:params {:c 100}} (:entity_id dash)))))))))) (deftest locked-params-are-substituted-into-text-cards (testing "check that locked params are substituted into text cards with mapped variables on the backend" @@ -595,7 +628,14 @@ :dashcards first :visualization_settings - :text))))))) + :text))) + (testing "with entity id" + (is (= "Text card with variable: bar" + (-> (client/client :get 200 (dashboard-url dash {:params {:a "bar"}} (:entity_id dash))) + :dashcards + first + :visualization_settings + :text)))))))) (deftest locked-params-removes-values-fields-and-mappings-test (testing "check that locked params are removed in parameter mappings, param_values, and param_fields" @@ -631,6 +671,21 @@ (get (mt/id :venues :name))))) (is (= 1 (-> embedding-dashboard + :dashcards + first + :parameter_mappings + count)))) + (let [eid-embedding-dashboard (client/client :get 200 (dashboard-url dashboard {:params {:foo "BCD Tofu House"}} (:entity_id dashboard)))] + (is (= nil + (-> eid-embedding-dashboard + :param_values + (get (mt/id :venues :name))))) + (is (= nil + (-> eid-embedding-dashboard + :param_fields + (get (mt/id :venues :name))))) + (is (= 1 + (-> eid-embedding-dashboard :dashcards first :parameter_mappings @@ -677,6 +732,21 @@ (get (mt/id :venues :name))))) (is (= 1 (-> embedding-dashboard + :dashcards + first + :parameter_mappings + count)))) + (let [eid-embedding-dashboard (client/client :get 200 (dashboard-url dashboard {:params {:foo "BCD Tofu House"}} (:entity_id dashboard)))] + (is (some? + (-> eid-embedding-dashboard + :param_values + (get (mt/id :venues :name))))) + (is (some? + (-> eid-embedding-dashboard + :param_fields + (get (mt/id :venues :name))))) + (is (= 1 + (-> eid-embedding-dashboard :dashcards first :parameter_mappings @@ -715,6 +785,12 @@ (let [embedding-dashboard (client/client :get 200 (dashboard-url dashboard {:params {:foo "BCD Tofu House"}}))] (is (= [] (-> embedding-dashboard + :param_values + (get (mt/id :categories :name)) + :values)))) + (let [eid-embedding-dashboard (client/client :get 200 (dashboard-url dashboard {:params {:foo "BCD Tofu House"}} (:entity_id dashboard)))] + (is (= [] + (-> eid-embedding-dashboard :param_values (get (mt/id :categories :name)) :values)))))))) @@ -723,17 +799,22 @@ (defn dashcard-url "The URL for a request to execute a query for a Card on an embedded Dashboard." - [dashcard & [additional-token-params]] - (str "embed/dashboard/" (dash-token (:dashboard_id dashcard) additional-token-params) + [dashcard & [additional-token-keys entity-id]] + (str "embed/dashboard/" (dash-token (:dashboard_id dashcard) additional-token-keys entity-id) "/dashcard/" (u/the-id dashcard) "/card/" (:card_id dashcard))) +(defn- dashcard->dash-eid [dashcard] + (t2/select-one-fn :entity_id :model/Dashboard :id (:dashboard_id dashcard))) + (deftest it-should-be-possible-to-run-a-card-successfully-if-you-jump-through-the-right-hoops--- (testing "it should be possible to run a Card successfully if you jump through the right hoops..." (with-embedding-enabled-and-new-secret-key! (with-temp-dashcard [dashcard {:dash {:enable_embedding true}}] #_{:clj-kondo/ignore [:deprecated-var]} - (test-query-results (client/client :get 202 (dashcard-url dashcard))))))) + (test-query-results (client/client :get 202 (dashcard-url dashcard))) + #_{:clj-kondo/ignore [:deprecated-var]} + (test-query-results (client/client :get 202 (dashcard-url dashcard {} (dashcard->dash-eid dashcard)))))))) (deftest downloading-csv-json-xlsx-results-from-the-dashcard-endpoint-shouldn-t-be-subject-to-the-default-query-constraints (testing (str "Downloading CSV/JSON/XLSX results from the dashcard endpoint shouldn't be subject to the default " @@ -747,7 +828,10 @@ :userland-query? true})}}] (let [results (client/client :get 200 (str (dashcard-url dashcard) "/csv"))] (is (= 101 - (count (csv/read-csv results)))))))))) + (count (csv/read-csv results))))) + (let [eid-results (client/client :get 200 (str (dashcard-url dashcard {} (dashcard->dash-eid dashcard)) "/csv"))] + (is (= 101 + (count (csv/read-csv eid-results)))))))))) (deftest embed-download-query-execution-test (testing "Tests that embedding download context shows up in the query execution table when downloading cards." @@ -805,7 +889,10 @@ {:name "PRICE" :fieldRef [:field (mt/id :venues :price) nil] :enabled true}]}}}] (let [results (client/client :get 200 (str (dashcard-url dashcard) "/csv"))] (is (= ["Name" "ID" "Category ID" "Price"] - (first (csv/read-csv results)))))))))) + (first (csv/read-csv results))))) + (let [eid-results (client/client :get 200 (str (dashcard-url dashcard {} (dashcard->dash-eid dashcard)) "/csv"))] + (is (= ["Name" "ID" "Category ID" "Price"] + (first (csv/read-csv eid-results)))))))))) (deftest generic-query-failed-exception-test (testing (str "...but if the card has an invalid query we should just get a generic \"query failed\" exception " @@ -957,20 +1044,20 @@ ;;; ------------------------------- GET /api/embed/card/:token/field/:field/values nil -------------------------------- -(defn- field-values-url [card-or-dashboard field-or-id] +(defn- field-values-url [card-or-dashboard field-or-id & [entity-id]] (str "embed/" (condp mi/instance-of? card-or-dashboard - Card (str "card/" (card-token card-or-dashboard)) - Dashboard (str "dashboard/" (dash-token card-or-dashboard))) + :model/Card (str "card/" (card-token card-or-dashboard {} entity-id)) + :model/Dashboard (str "dashboard/" (dash-token card-or-dashboard {} entity-id))) "/field/" (u/the-id field-or-id) "/values")) (defn- do-with-embedding-enabled-and-temp-card-referencing! [table-kw field-kw f] (with-embedding-enabled-and-new-secret-key! - (t2.with-temp/with-temp [Card card (assoc (public-test/mbql-card-referencing table-kw field-kw) - :enable_embedding true)] + (t2.with-temp/with-temp [:model/Card card (assoc (public-test/mbql-card-referencing table-kw field-kw) + :enable_embedding true)] (f card)))) (defmacro ^:private with-embedding-enabled-and-temp-card-referencing! @@ -1015,20 +1102,20 @@ (client/client :get 400 (field-values-url card (mt/id :venues :name))))))) (deftest card-param-values - (letfn [(search [card param-key prefix] + (letfn [(search [card param-key prefix & [entity-id]] (client/client :get 200 (format "embed/card/%s/params/%s/search/%s" - (card-token card) param-key prefix))) - (dropdown [card param-key] + (card-token card nil entity-id) param-key prefix))) + (dropdown [card param-key & [entity-id]] (client/client :get 200 (format "embed/card/%s/params/%s/values" - (card-token card) param-key)))] + (card-token card nil entity-id) param-key)))] (mt/with-temporary-setting-values [enable-embedding true] (with-new-secret-key! (api.card-test/with-card-param-values-fixtures [{:keys [card field-filter-card param-keys]}] - (t2/update! Card (:id field-filter-card) + (t2/update! :model/Card (:id field-filter-card) {:enable_embedding true :embedding_params (zipmap (map :slug (:parameters field-filter-card)) (repeat "enabled"))}) - (t2/update! Card (:id card) + (t2/update! :model/Card (:id card) {:enable_embedding true :embedding_params (zipmap (map :slug (:parameters card)) (repeat "enabled"))}) @@ -1037,10 +1124,20 @@ (is (false? (:has_more_values response))) (is (set/subset? #{["20th Century Cafe"] ["33 Taps"]} (-> response :values set)))) + (let [response (search field-filter-card (:field-values param-keys) "bar")] (is (set/subset? #{["Barney's Beanery"] ["bigmista's barbecue"]} (-> response :values set))) (is (not ((into #{} (mapcat identity) (:values response)) "The Virgil"))))) + (testing "field filter based param entity-id" + (let [response (dropdown field-filter-card (:field-values param-keys) (:entity_id field-filter-card))] + (is (false? (:has_more_values response))) + (is (set/subset? #{["20th Century Cafe"] ["33 Taps"]} + (-> response :values set)))) + (let [response (search field-filter-card (:field-values param-keys) "bar" (:entity_id field-filter-card))] + (is (set/subset? #{["Barney's Beanery"] ["bigmista's barbecue"]} + (-> response :values set))) + (is (not ((into #{} (mapcat identity) (:values response)) "The Virgil"))))) (testing "static based param" (let [response (dropdown card (:static-list param-keys))] (is (= {:has_more_values false, @@ -1050,6 +1147,15 @@ (is (= {:has_more_values false, :values [["African"]]} response)))) + (testing "static based param entity-id" + (let [response (dropdown card (:static-list param-keys) (:entity_id card))] + (is (= {:has_more_values false, + :values [["African"] ["American"] ["Asian"]]} + response))) + (let [response (search card (:static-list param-keys) "af" (:entity_id card))] + (is (= {:has_more_values false, + :values [["African"]]} + response)))) (testing "card based param" (let [response (dropdown card (:card param-keys))] (is (= {:values [["20th Century Cafe"] ["25°"] ["33 Taps"] @@ -1057,6 +1163,16 @@ :has_more_values false} response))) (let [response (search card (:card param-keys) "red")] + (is (= {:has_more_values false, + :values [["Fred 62"] ["Red Medicine"]]} + response)))) + (testing "card based param entity-id" + (let [response (dropdown card (:card param-keys) (:entity_id card))] + (is (= {:values [["20th Century Cafe"] ["25°"] ["33 Taps"] + ["800 Degrees Neapolitan Pizzeria"] ["BCD Tofu House"]] + :has_more_values false} + response))) + (let [response (search card (:card param-keys) "red" (:entity_id card))] (is (= {:has_more_values false, :values [["Fred 62"] ["Red Medicine"]]} response))))))))) @@ -1065,14 +1181,14 @@ (defn- do-with-embedding-enabled-and-temp-dashcard-referencing! [table-kw field-kw f] (with-embedding-enabled-and-new-secret-key! - (mt/with-temp [Dashboard dashboard {:enable_embedding true} - Card card (public-test/mbql-card-referencing table-kw field-kw) - DashboardCard dashcard {:dashboard_id (u/the-id dashboard) - :card_id (u/the-id card) - :parameter_mappings [{:card_id (u/the-id card) - :target [:dimension - [:field - (mt/id table-kw field-kw) nil]]}]}] + (mt/with-temp [:model/Dashboard dashboard {:enable_embedding true} + :model/Card card (public-test/mbql-card-referencing table-kw field-kw) + :model/DashboardCard dashcard {:dashboard_id (u/the-id dashboard) + :card_id (u/the-id card) + :parameter_mappings [{:card_id (u/the-id card) + :target [:dimension + [:field + (mt/id table-kw field-kw) nil]]}]}] (f dashboard card dashcard)))) (defmacro ^:private with-embedding-enabled-and-temp-dashcard-referencing! @@ -1094,6 +1210,9 @@ :has_more_values false} (with-embedding-enabled-and-temp-dashcard-referencing! :venues :name [dashboard] (-> (client/client :get 200 (field-values-url dashboard (mt/id :venues :name))) + (update :values (partial take 5)))) + (with-embedding-enabled-and-temp-dashcard-referencing! :venues :name [dashboard] + (-> (client/client :get 200 (field-values-url dashboard (mt/id :venues :name) (:entity_id dashboard))) (update :values (partial take 5))))))) ;; shound NOT be able to use the endpoint with a Field not referenced by the Dashboard @@ -1118,11 +1237,11 @@ ;;; --------------------------------------------- Field search endpoints --------------------------------------------- -(defn- field-search-url [card-or-dashboard field-or-id search-field-or-id] +(defn- field-search-url [card-or-dashboard field-or-id search-field-or-id & [entity-id]] (str "embed/" (condp mi/instance-of? card-or-dashboard - Card (str "card/" (card-token card-or-dashboard)) - Dashboard (str "dashboard/" (dash-token card-or-dashboard))) + Card (str "card/" (card-token card-or-dashboard {} entity-id)) + Dashboard (str "dashboard/" (dash-token card-or-dashboard {} entity-id))) "/field/" (u/the-id field-or-id) "/search/" (u/the-id search-field-or-id))) @@ -1132,6 +1251,9 @@ (is (= [[93 "33 Taps"]] (client/client :get 200 (field-search-url object (mt/id :venues :id) (mt/id :venues :name)) :value "33 T"))) + (is (= [[93 "33 Taps"]] + (client/client :get 200 (field-search-url object (mt/id :venues :id) (mt/id :venues :name) (:entity_id object)) + :value "33 T"))) (testing "if search field isn't allowed to be used with the other Field endpoint should return exception" (is (= "Invalid Request." @@ -1152,50 +1274,58 @@ (testing "GET /api/embed/card/:token/field/:field/search/:search-field-id nil" (testing "Search for Field values for a Card" (with-embedding-enabled-and-temp-card-referencing! :venues :id [card] - (tests Card card)))) + (tests :model/Card card)))) (testing "GET /api/embed/dashboard/:token/field/:field/search/:search-field-id nil" (testing "Search for Field values for a Dashboard" (with-embedding-enabled-and-temp-dashcard-referencing! :venues :id [dashboard] - (tests Dashboard dashboard))))))) + (tests :model/Dashboard dashboard))))))) ;;; ----------------------- GET /api/embed/card/:token/field/:field/remapping/:remapped-id nil ------------------------ -(defn- field-remapping-url [card-or-dashboard field-or-id remapped-field-or-id] +(defn- field-remapping-url [card-or-dashboard field-or-id remapped-field-or-id & [entity-id]] (str "embed/" (condp mi/instance-of? card-or-dashboard - Card (str "card/" (card-token card-or-dashboard)) - Dashboard (str "dashboard/" (dash-token card-or-dashboard))) + Card (str "card/" (card-token card-or-dashboard {} entity-id)) + Dashboard (str "dashboard/" (dash-token card-or-dashboard {} entity-id))) "/field/" (u/the-id field-or-id) "/remapping/" (u/the-id remapped-field-or-id))) (deftest field-remapping-test - (letfn [(tests [model object] + (letfn [(tests [model object & [entity-id]] (testing (str "we should be able to use the API endpoint and get the same results we get by calling the " "function above directly") (is (= [10 "Fred 62"] - (client/client :get 200 (field-remapping-url object (mt/id :venues :id) (mt/id :venues :name)) + (client/client :get 200 (field-remapping-url + object (mt/id :venues :id) (mt/id :venues :name) entity-id) :value "10")))) + (testing " ...or if the remapping Field isn't allowed to be used with the other Field" (is (= "Invalid Request." - (client/client :get 400 (field-remapping-url object (mt/id :venues :id) (mt/id :venues :price)) + (client/client :get 400 (field-remapping-url + object (mt/id :venues :id) (mt/id :venues :price) entity-id) :value "10")))) (testing " ...or if embedding is disabled" (mt/with-temporary-setting-values [enable-embedding false] (is (= "Embedding is not enabled." - (client/client :get 400 (field-remapping-url object (mt/id :venues :id) (mt/id :venues :name)) + (client/client :get 400 (field-remapping-url + object (mt/id :venues :id) (mt/id :venues :name) entity-id) :value "10"))))) (testing " ...or if embedding is disabled for the Card/Dashboard" (t2/update! model (u/the-id object) {:enable_embedding false}) (is (= "Embedding is not enabled for this object." - (client/client :get 400 (field-remapping-url object (mt/id :venues :id) (mt/id :venues :name)) + (client/client :get 400 (field-remapping-url + object (mt/id :venues :id) (mt/id :venues :name) entity-id) :value "10")))))] (testing "GET /api/embed/card/:token/field/:field/remapping/:remapped-id nil" (testing "Get remapped Field values for a Card" (with-embedding-enabled-and-temp-card-referencing! :venues :id [card] - (tests Card card))) + (tests :model/Card card))) + (testing "Get remapped Field values for a Card with entity-ids" + (with-embedding-enabled-and-temp-card-referencing! :venues :id [card] + (tests :model/Card card (:entity_id card)))) (testing "Shouldn't work if Card doesn't reference the Field in question" (with-embedding-enabled-and-temp-card-referencing! :venues :price [card] (is (= "Not found." @@ -1205,12 +1335,42 @@ (testing "GET /api/embed/dashboard/:token/field/:field/remapping/:remapped-id nil" (testing "Get remapped Field values for a Dashboard" (with-embedding-enabled-and-temp-dashcard-referencing! :venues :id [dashboard] - (tests Dashboard dashboard))) + (tests :model/Dashboard dashboard))) + (testing "Get remapped Field values for a Dashboard with entity-ids" + (with-embedding-enabled-and-temp-dashcard-referencing! :venues :id [dashboard] + (tests :model/Dashboard dashboard (:entity_id dashboard)))) (testing "Shouldn't work if Dashboard doesn't reference the Field in question" (with-embedding-enabled-and-temp-dashcard-referencing! :venues :price [dashboard] (is (= "Not found." (client/client :get 400 (field-remapping-url dashboard (mt/id :venues :id) (mt/id :venues :name)) - :value "10")))))))) + :value "10")))))) + + (testing "with entity ids" + (testing "GET /api/embed/card/:token/field/:field/remapping/:remapped-id nil" + (testing "Get remapped Field values for a Card" + (with-embedding-enabled-and-temp-card-referencing! :venues :id [card] + (tests :model/Card card (:entity_id card)))) + (testing "Get remapped Field values for a Card with entity-ids" + (with-embedding-enabled-and-temp-card-referencing! :venues :id [card] + (tests :model/Card card (:entity_id card)))) + (testing "Shouldn't work if Card doesn't reference the Field in question" + (with-embedding-enabled-and-temp-card-referencing! :venues :price [card] + (is (= "Not found." + (client/client :get 400 (field-remapping-url card (mt/id :venues :id) (mt/id :venues :name)) + :value "10")))))) + + (testing "GET /api/embed/dashboard/:token/field/:field/remapping/:remapped-id nil" + (testing "Get remapped Field values for a Dashboard" + (with-embedding-enabled-and-temp-dashcard-referencing! :venues :id [dashboard] + (tests :model/Dashboard dashboard (:entity_id dashboard)))) + (testing "Get remapped Field values for a Dashboard with entity-ids" + (with-embedding-enabled-and-temp-dashcard-referencing! :venues :id [dashboard] + (tests :model/Dashboard dashboard (:entity_id dashboard)))) + (testing "Shouldn't work if Dashboard doesn't reference the Field in question" + (with-embedding-enabled-and-temp-dashcard-referencing! :venues :price [dashboard] + (is (= "Not found." + (client/client :get 400 (field-remapping-url dashboard (mt/id :venues :id) (mt/id :venues :name)) + :value "10"))))))))) ;;; ------------------------------------------------ Chain filtering ------------------------------------------------- @@ -1395,9 +1555,9 @@ ;; Pivot tables -(defn- pivot-card-query-url [card response-format & [additional-token-params]] +(defn- pivot-card-query-url [card-or-id response-format & [additional-token-keys]] (str "/embed/pivot/card/" - (card-token card additional-token-params) + (card-token card-or-id additional-token-keys) "/query" response-format)) @@ -1421,7 +1581,15 @@ (is (nil? (:row_count result))) ;; row_count isn't included in public endpoints (is (= "completed" (:status result))) (is (= 6 (count (get-in result [:data :cols])))) - (is (= 1144 (count rows))))))) + (is (= 1144 (count rows)))) + (let [eid-result (client/client :get expected-status + (pivot-card-query-url (:entity_id card) "") + {:request-options nil}) + eid-rows (mt/rows eid-result)] + (is (nil? (:row_count eid-result))) ;; row_count isn't included in public endpoints + (is (= "completed" (:status eid-result))) + (is (= 6 (count (get-in eid-result [:data :cols])))) + (is (= 1144 (count eid-rows))))))) (testing "check that if embedding *is* enabled globally but not for the Card the request fails" (with-temp-card [card (api.pivots/pivot-card)] @@ -1434,10 +1602,12 @@ (is (= "Message seems corrupt or manipulated" (client/client :get 400 (with-new-secret-key! (pivot-card-query-url card "")))))))))))) -(defn- pivot-dashcard-url [dashcard & [additional-token-params]] - (str "embed/pivot/dashboard/" (dash-token (:dashboard_id dashcard) additional-token-params) - "/dashcard/" (u/the-id dashcard) - "/card/" (:card_id dashcard))) +(defn- pivot-dashcard-url + ([dashcard] (pivot-dashcard-url dashcard (:dashboard_id dashcard))) + ([dashcard dashboard-id & [additional-token-keys]] + (str "embed/pivot/dashboard/" (dash-token dashboard-id additional-token-keys) + "/dashcard/" (u/the-id dashcard) + "/card/" (:card_id dashcard)))) (deftest pivot-dashcard-success-test (mt/test-drivers (api.pivots/applicable-drivers) @@ -1446,12 +1616,18 @@ (with-temp-dashcard [dashcard {:dash {:enable_embedding true, :parameters []} :card (api.pivots/pivot-card) :dashcard {:parameter_mappings []}}] - (let [result (client/client :get 202 (pivot-dashcard-url dashcard)) + (let [result (client/client :get 202 (pivot-dashcard-url dashcard (:dashboard_id dashcard))) rows (mt/rows result)] (is (nil? (:row_count result))) ;; row_count isn't included in public endpoints (is (= "completed" (:status result))) (is (= 6 (count (get-in result [:data :cols])))) - (is (= 1144 (count rows))))))))) + (is (= 1144 (count rows)))) + (let [eid-result (client/client :get 202 (pivot-dashcard-url dashcard (dashcard->dash-eid dashcard))) + eid-rows (mt/rows eid-result)] + (is (nil? (:row_count eid-result))) ;; row_count isn't included in public endpoints + (is (= "completed" (:status eid-result))) + (is (= 6 (count (get-in eid-result [:data :cols])))) + (is (= 1144 (count eid-rows))))))))) (deftest pivot-dashcard-embedding-disabled-test (mt/dataset test-data @@ -1502,12 +1678,19 @@ (client/client :get 400 (pivot-dashcard-url dashcard))))) (testing "if `:locked` param is supplied, request should succeed" - (let [result (client/client :get 202 (pivot-dashcard-url dashcard {:params {:abc 100}})) + (let [result (client/client :get 202 (pivot-dashcard-url dashcard (:dashboard_id dashcard) {:params {:abc 100}})) rows (mt/rows result)] (is (nil? (:row_count result))) ;; row_count isn't included in public endpoints (is (= "completed" (:status result))) (is (= 6 (count (get-in result [:data :cols])))) (is (= 1144 (count rows))))) + (testing "if `:locked` param is supplied, request should succeed with entity-id" + (let [eid-result (client/client :get 202 (pivot-dashcard-url dashcard (dashcard->dash-eid dashcard) {:params {:abc 100}})) + eid-rows (mt/rows eid-result)] + (is (nil? (:row_count eid-result))) ;; row_count isn't included in public endpoints + (is (= "completed" (:status eid-result))) + (is (= 6 (count (get-in eid-result [:data :cols])))) + (is (= 1144 (count eid-rows))))) (testing "if `:locked` parameter is present in URL params, request should fail" (is (= "You must specify a value for :abc in the JWT." @@ -1524,7 +1707,7 @@ (testing (str "check that if embedding is enabled globally and for the object requests fail if they pass a " "`:disabled` parameter") (is (= "You're not allowed to specify a value for :abc." - (client/client :get 400 (pivot-dashcard-url dashcard {:params {:abc 100}}))))) + (client/client :get 400 (pivot-dashcard-url dashcard (:dashboard_id dashcard) {:params {:abc 100}}))))) (testing "If a `:disabled` param is passed in the URL the request should fail" (is (= "You're not allowed to specify a value for :abc." @@ -1544,16 +1727,24 @@ :dashcard {:parameter_mappings []}}] (testing "If `:enabled` param is present in both JWT and the URL, the request should fail" (is (= "You can't specify a value for :abc if it's already set in the JWT." - (client/client :get 400 (str (pivot-dashcard-url dashcard {:params {:abc 100}}) "?abc=200"))))) + (client/client :get 400 (str (pivot-dashcard-url dashcard (:dashboard_id dashcard) {:params {:abc 100}}) "?abc=200"))))) (testing "If an `:enabled` param is present in the JWT, that's ok" - (let [result (client/client :get 202 (pivot-dashcard-url dashcard {:params {:abc 100}})) + (let [result (client/client :get 202 (pivot-dashcard-url dashcard (:dashboard_id dashcard) {:params {:abc 100}})) rows (mt/rows result)] (is (nil? (:row_count result))) ;; row_count isn't included in public endpoints (is (= "completed" (:status result))) (is (= 6 (count (get-in result [:data :cols])))) (is (= 1144 (count rows))))) + (testing "If an `:enabled` param is present in the JWT, that's ok with entity-id" + (let [eid-result (client/client :get 202 (pivot-dashcard-url dashcard (dashcard->dash-eid dashcard) {:params {:abc 100}})) + eid-rows (mt/rows eid-result)] + (is (nil? (:row_count eid-result))) ;; row_count isn't included in public endpoints + (is (= "completed" (:status eid-result))) + (is (= 6 (count (get-in eid-result [:data :cols])))) + (is (= 1144 (count eid-rows))))) + (testing "If an `:enabled` param is present in URL params but *not* the JWT, that's ok" (let [result (client/client :get 202 (str (pivot-dashcard-url dashcard) "?abc=200")) rows (mt/rows result)] @@ -1745,8 +1936,8 @@ (deftest entity-id-single-card-translations-test (mt/with-temp - [:model/Card {id :id eid :entity_id} {}] - (is (= {eid {:id id :type :card}} + [:model/Card {id :id eid :entity_id} {}] + (is (= {eid {:id id :type :card :status "ok"}} (api.embed.common/model->entity-ids->ids {:card [eid]}))))) (deftest entity-id-card-translations-test @@ -1758,13 +1949,13 @@ :model/Card {id-3 :id eid-3 :entity_id} {} :model/Card {id-4 :id eid-4 :entity_id} {} :model/Card {id-5 :id eid-5 :entity_id} {}] - (is (= {eid {:id id :type :card} - eid-0 {:id id-0 :type :card} - eid-1 {:id id-1 :type :card} - eid-2 {:id id-2 :type :card} - eid-3 {:id id-3 :type :card} - eid-4 {:id id-4 :type :card} - eid-5 {:id id-5 :type :card}} + (is (= {eid {:id id :type :card :status "ok"} + eid-0 {:id id-0 :type :card :status "ok"} + eid-1 {:id id-1 :type :card :status "ok"} + eid-2 {:id id-2 :type :card :status "ok"} + eid-3 {:id id-3 :type :card :status "ok"} + eid-4 {:id id-4 :type :card :status "ok"} + eid-5 {:id id-5 :type :card :status "ok"}} (api.embed.common/model->entity-ids->ids {:card [eid eid-0 eid-1 eid-2 eid-3 eid-4 eid-5]}))))) (deftest entity-id-mixed-translations-test @@ -1785,35 +1976,35 @@ :model/Pulse {pulse_id :id pulse_eid :entity_id} {} :model/PulseCard {pulse_card_id :id pulse_card_eid :entity_id} {:pulse_id pulse_id :card_id card-id} :model/PulseChannel {pulse_channel_id :id pulse_channel_eid :entity_id} {:pulse_id pulse_id} - :model/Card {report_card_id :id report_card_eid :entity_id} {} - :model/Dashboard {report_dashboard_id :id report_dashboard_eid :entity_id} {} - :model/DashboardTab {dashboard_tab_id :id dashboard_tab_eid :entity_id} {:dashboard_id report_dashboard_id} - :model/DashboardCard {report_dashboardcard_id :id report_dashboardcard_eid :entity_id} {:dashboard_id report_dashboard_id} + :model/Card {card_id :id card_eid :entity_id} {} + :model/Dashboard {dashboard_id :id dashboard_eid :entity_id} {} + :model/DashboardTab {dashboard_tab_id :id dashboard_tab_eid :entity_id} {:dashboard_id dashboard_id} + :model/DashboardCard {dashboardcard_id :id dashboardcard_eid :entity_id} {:dashboard_id dashboard_id} :model/Segment {segment_id :id segment_eid :entity_id} {} :model/Timeline {timeline_id :id timeline_eid :entity_id} {}] (let [core_user_eid (u/generate-nano-id)] (t2/update! :model/User core_user_id {:entity_id core_user_eid}) - (is (= {action_eid {:id action_id :type :action} - collection_eid {:id collection_id :type :collection} - core_user_eid {:id core_user_id :type :user} - dashboard_tab_eid {:id dashboard_tab_id :type :dashboard-tab} - dimension_eid {:id dimension_id :type :dimension} - native_query_snippet_eid {:id native_query_snippet_id :type :snippet} - permissions_group_eid {:id permissions_group_id :type :permissions-group} - pulse_eid {:id pulse_id :type :pulse} - pulse_card_eid {:id pulse_card_id :type :pulse-card} - pulse_channel_eid {:id pulse_channel_id :type :pulse-channel} - report_card_eid {:id report_card_id :type :card} - report_dashboard_eid {:id report_dashboard_id :type :dashboard} - report_dashboardcard_eid {:id report_dashboardcard_id :type :dashboard-card} - segment_eid {:id segment_id :type :segment} - timeline_eid {:id timeline_id :type :timeline}} + (is (= {action_eid {:id action_id :type :action :status "ok"} + collection_eid {:id collection_id :type :collection :status "ok"} + core_user_eid {:id core_user_id :type :user :status "ok"} + dashboard_tab_eid {:id dashboard_tab_id :type :dashboard-tab :status "ok"} + dimension_eid {:id dimension_id :type :dimension :status "ok"} + native_query_snippet_eid {:id native_query_snippet_id :type :snippet :status "ok"} + permissions_group_eid {:id permissions_group_id :type :permissions-group :status "ok"} + pulse_eid {:id pulse_id :type :pulse :status "ok"} + pulse_card_eid {:id pulse_card_id :type :pulse-card :status "ok"} + pulse_channel_eid {:id pulse_channel_id :type :pulse-channel :status "ok"} + card_eid {:id card_id :type :card :status "ok"} + dashboard_eid {:id dashboard_id :type :dashboard :status "ok"} + dashboardcard_eid {:id dashboardcard_id :type :dashboard-card :status "ok"} + segment_eid {:id segment_id :type :segment :status "ok"} + timeline_eid {:id timeline_id :type :timeline :status "ok"}} (api.embed.common/model->entity-ids->ids {:action [action_eid] - :card [report_card_eid] + :card [card_eid] :collection [collection_eid] - :dashboard [report_dashboard_eid] - :dashboard-card [report_dashboardcard_eid] + :dashboard [dashboard_eid] + :dashboard-card [dashboardcard_eid] :dashboard-tab [dashboard_tab_eid] :dimension [dimension_eid] :permissions-group [permissions_group_eid] @@ -1828,3 +2019,10 @@ (deftest missing-entity-translations-test (is (= {"abcdefghijklmnopqrstu" {:type :card, :status "not-found"}} (api.embed.common/model->entity-ids->ids {:card ["abcdefghijklmnopqrstu"]})))) + +(deftest wrong-format-entity-translations-test + (is (= {"abcdefghijklmnopqrst" + {:type :card, + :status "invalid-format", + :reason ["\"abcdefghijklmnopqrst\" should be 21 characters long, but it is 20"]}} + (api.embed.common/model->entity-ids->ids {:card ["abcdefghijklmnopqrst"]})))) diff --git a/test/metabase/api/util_test.clj b/test/metabase/api/util_test.clj index 58e3d1545edffc60215f54c3bbc3d9790a7d9e81..346c3641567e8407c0da72e5ebc0fa76d3e15d6d 100644 --- a/test/metabase/api/util_test.clj +++ b/test/metabase/api/util_test.clj @@ -84,7 +84,7 @@ (deftest entity-id-translation-test (mt/with-temp [:model/Card {card-id :id card-eid :entity_id} {}] - (is (= {card-eid {:id card-id :type "card"}} + (is (= {card-eid {:id card-id :type "card" :status "ok"}} (-> (mt/user-http-request :crowberto :post 200 "util/entity_id" {:entity_ids {"card" [card-eid]}})