diff --git a/frontend/src/metabase/dashboard/components/add-card-sidebar/QuestionList.jsx b/frontend/src/metabase/dashboard/components/add-card-sidebar/QuestionList.jsx index 470a3eeca918a0d04ba22f2db4598e01955ee0dd..0395a9e4ddcceda9249698fad06634c5346d7d58 100644 --- a/frontend/src/metabase/dashboard/components/add-card-sidebar/QuestionList.jsx +++ b/frontend/src/metabase/dashboard/components/add-card-sidebar/QuestionList.jsx @@ -15,6 +15,8 @@ QuestionList.propTypes = { hasCollections: PropTypes.bool, }; +const SEARCH_LIMIT = 1000; + export function QuestionList({ searchText, collectionId, @@ -35,6 +37,7 @@ export function QuestionList({ query = { ...query, models: "card", + limit: SEARCH_LIMIT, }; return ( diff --git a/frontend/src/metabase/nav/components/SearchBar.jsx b/frontend/src/metabase/nav/components/SearchBar.jsx index 6437003376f4896dfa0a729c07760b12dabf7a45..35943215357cf31ac9f0185ec953bcc1c1407fb3 100644 --- a/frontend/src/metabase/nav/components/SearchBar.jsx +++ b/frontend/src/metabase/nav/components/SearchBar.jsx @@ -52,6 +52,7 @@ const SearchInput = styled.input` `; const ALLOWED_SEARCH_FOCUS_ELEMENTS = new Set(["BODY", "A"]); +const SEARCH_LIMIT = 50; export default class SearchBar extends React.Component { state = { @@ -152,7 +153,7 @@ export default class SearchBar extends React.Component { py={1} > <Search.ListLoader - query={{ q: searchText.trim() }} + query={{ q: searchText.trim(), limit: SEARCH_LIMIT }} wrapped reload debounced diff --git a/frontend/src/metabase/query_builder/components/data-search/SearchResults.jsx b/frontend/src/metabase/query_builder/components/data-search/SearchResults.jsx index 60e31ff01141603ccfdde9dec8150eba68b2783e..e5973072fb2596c33c0b91762d861f68831c15ee 100644 --- a/frontend/src/metabase/query_builder/components/data-search/SearchResults.jsx +++ b/frontend/src/metabase/query_builder/components/data-search/SearchResults.jsx @@ -18,6 +18,8 @@ const propTypes = { ), }; +const SEARCH_LIMIT = 1000; + export function SearchResults({ searchQuery, onSelect, @@ -27,6 +29,7 @@ export function SearchResults({ const query = { q: searchQuery, models: searchModels, + limit: SEARCH_LIMIT, }; if (databaseId) { diff --git a/src/metabase/api/field.clj b/src/metabase/api/field.clj index d92c02cbf359256ccb4b6f534611995cece571d3..71209a9e3a39808e42938e4c8175fa1aad4a2f15 100644 --- a/src/metabase/api/field.clj +++ b/src/metabase/api/field.clj @@ -12,6 +12,7 @@ [metabase.models.table :as table :refer [Table]] [metabase.query-processor :as qp] [metabase.related :as related] + [metabase.server.middleware.offset-paging :as offset-paging] [metabase.types :as types] [metabase.util :as u] [metabase.util.i18n :refer [trs]] @@ -21,8 +22,11 @@ [toucan.hydrate :refer [hydrate]]) (:import java.text.NumberFormat)) + ;;; --------------------------------------------- Basic CRUD Operations ---------------------------------------------- +(def ^:private default-max-field-search-limit 1000) + (def ^:private FieldVisibilityType "Schema for a valid `Field` visibility type." (apply s/enum (map name field/visibility-types))) @@ -137,7 +141,6 @@ ;; return updated field (hydrate (Field id) :dimensions))) - ;;; ------------------------------------------------- Field Metadata ------------------------------------------------- (api/defendpoint GET "/:id/summary" @@ -297,6 +300,7 @@ (db/select-one Field :id fk-target-field-id) field)) + (defn- search-values-query "Generate the MBQL query used to power FieldValues search in `search-values` below. The actual query generated differs slightly based on whether the two Fields are the same Field." @@ -326,9 +330,10 @@ ;; -> ((14 \"Marilyne Mohr\") (36 \"Margot Farrell\") (48 \"Maryam Douglas\"))" - [field search-field value & [limit]] + [field search-field value maybe-limit] (try (let [field (follow-fks field) + limit (or maybe-limit default-max-field-search-limit) results (qp/process-query (search-values-query field search-field value limit)) rows (get-in results [:data :rows])] ;; if the two Fields are different, we'll get results like [[v1 v2] [v1 v2]]. That is the expected format and we can @@ -344,17 +349,17 @@ (log/debug e (trs "Error searching field values")) nil))) + (api/defendpoint GET "/:id/search/:search-id" "Search for values of a Field with `search-id` that start with `value`. See docstring for `metabase.api.field/search-values` for a more detailed explanation." - [id search-id value limit] - {value su/NonBlankString - limit (s/maybe su/IntStringGreaterThanZero)} + [id search-id value] + {value su/NonBlankString} (let [field (api/check-404 (Field id)) search-field (api/check-404 (Field search-id))] (throw-if-no-read-or-segmented-perms field) (throw-if-no-read-or-segmented-perms search-field) - (search-values field search-field value (when limit (Integer/parseInt limit))))) + (search-values field search-field value offset-paging/*limit*))) (defn remapped-value "Search for one specific remapping where the value of `field` exactly matches `value`. Returns a pair like diff --git a/test/metabase/api/field_test.clj b/test/metabase/api/field_test.clj index 53543135c8790fa3516c44fbf3f884a67f708242..00235007c8d56b65855bad8358ac0bb64814de81 100644 --- a/test/metabase/api/field_test.clj +++ b/test/metabase/api/field_test.clj @@ -595,7 +595,8 @@ (mt/format-rows-by [int str] (field-api/search-values (Field (mt/id :venues :id)) (Field (mt/id :venues :name)) - "Red"))))) + "Red" + nil))))) (tqp.test/test-timeseries-drivers (is (= [["139" "Red Medicine"] ["148" "Fred 62"] @@ -608,7 +609,16 @@ ["977" "Fred 62"]] (field-api/search-values (Field (mt/id :checkins :id)) (Field (mt/id :checkins :venue_name)) - "Red")))))) + "Red" + nil))))) + (testing "make sure limit works" + (mt/test-drivers (mt/normal-drivers) + (is (= [[1 "Red Medicine"]] + (mt/format-rows-by [int str] + (field-api/search-values (Field (mt/id :venues :id)) + (Field (mt/id :venues :name)) + "Red" + 1))))))) (deftest search-values-with-field-same-as-search-field-test (testing "make sure it also works if you use the same Field twice" @@ -616,9 +626,11 @@ (is (= [["Fred 62" "Fred 62"] ["Red Medicine" "Red Medicine"]] (field-api/search-values (Field (mt/id :venues :name)) (Field (mt/id :venues :name)) - "Red")))) + "Red" + nil)))) (tqp.test/test-timeseries-drivers (is (= [["Fred 62" "Fred 62"] ["Red Medicine" "Red Medicine"]] (field-api/search-values (Field (mt/id :checkins :venue_name)) (Field (mt/id :checkins :venue_name)) - "Red")))))) + "Red" + nil)))))) diff --git a/test/metabase/api/search_test.clj b/test/metabase/api/search_test.clj index 1b4f0776356d9af7ca6f665d0482768ece90e8c6..ad072e50023d76d91f5f76829f4f586317beb441 100644 --- a/test/metabase/api/search_test.clj +++ b/test/metabase/api/search_test.clj @@ -222,6 +222,12 @@ (testing "It offsets matches properly" (with-search-items-in-root-collection "test" (is (<= 4 (count (search-request-data :crowberto :q "test" :limit "100" :offset "2")))))) + (testing "It offsets without limit properly" + (with-search-items-in-root-collection "test" + (is (= 4 (count (search-request-data :crowberto :q "test" :offset "2")))))) + (testing "It limits without offset properly" + (with-search-items-in-root-collection "test" + (is (= 2 (count (search-request-data :crowberto :q "test" :limit "2")))))) (testing "It subsets matches for model" (with-search-items-in-root-collection "test" (is (= 0 (count (search-request-data :crowberto :q "test" :models "database"))))