From d438e16d03d63f8a457c32ba0ba905d8dfd752a3 Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Tue, 21 Mar 2023 16:38:14 +0200
Subject: [PATCH] Fix data picker search (#29325)
---
.../ActionDashcardSettings.unit.spec.tsx | 5 +-
.../containers/DataPicker/DataPickerView.tsx | 1 -
.../DataPicker/DataSearch/DataSearch.tsx | 11 +--
.../tests/DataPicker-Models.unit.spec.ts | 51 ++++++++++++
.../tests/DataPicker-Questions.unit.spec.ts | 51 +++++++++++-
.../containers/DataPicker/tests/common.tsx | 77 +++++++++++++------
.../data-search/SearchResults.jsx | 4 +-
.../test/__support__/server-mocks/search.ts | 38 +++++----
8 files changed, 183 insertions(+), 55 deletions(-)
diff --git a/frontend/src/metabase/actions/components/ActionViz/ActionDashcardSettings.unit.spec.tsx b/frontend/src/metabase/actions/components/ActionViz/ActionDashcardSettings.unit.spec.tsx
index e69154bcd5e..bb20405c4cb 100644
--- a/frontend/src/metabase/actions/components/ActionViz/ActionDashcardSettings.unit.spec.tsx
+++ b/frontend/src/metabase/actions/components/ActionViz/ActionDashcardSettings.unit.spec.tsx
@@ -84,9 +84,12 @@ const setup = (
React.ComponentProps<typeof ConnectedActionDashcardSettings>
>,
) => {
+ const searchItems = models.map(model =>
+ createMockCollectionItem({ ...model, model: "dataset" }),
+ );
const closeSpy = jest.fn();
- setupSearchEndpoints(models.map(model => createMockCollectionItem(model)));
+ setupSearchEndpoints(searchItems);
setupCardsEndpoints(models);
setupActionsEndpoints([...actions1, ...actions2]);
diff --git a/frontend/src/metabase/containers/DataPicker/DataPickerView.tsx b/frontend/src/metabase/containers/DataPicker/DataPickerView.tsx
index 1a6267c6862..a0b0e6fede2 100644
--- a/frontend/src/metabase/containers/DataPicker/DataPickerView.tsx
+++ b/frontend/src/metabase/containers/DataPicker/DataPickerView.tsx
@@ -48,7 +48,6 @@ function DataPickerViewContent({
if (searchQuery.trim().length > MIN_SEARCH_LENGTH) {
return (
<DataSearch
- value={value}
searchQuery={searchQuery}
availableDataTypes={availableDataTypes}
onChange={onChange}
diff --git a/frontend/src/metabase/containers/DataPicker/DataSearch/DataSearch.tsx b/frontend/src/metabase/containers/DataPicker/DataSearch/DataSearch.tsx
index 8bc6ecda2a1..742f21ad8bb 100644
--- a/frontend/src/metabase/containers/DataPicker/DataSearch/DataSearch.tsx
+++ b/frontend/src/metabase/containers/DataPicker/DataSearch/DataSearch.tsx
@@ -16,7 +16,6 @@ import { useDataPicker } from "../DataPickerContext";
import type { DataPickerValue, DataPickerDataType } from "../types";
interface DataSearchProps {
- value: DataPickerValue;
searchQuery: string;
availableDataTypes: DataPickerDataType[];
onChange: (value: DataPickerValue) => void;
@@ -68,7 +67,7 @@ function getValueForVirtualTable(table: TableSearchResult): DataPickerValue {
isDatasets: type === "models",
});
return {
- type: "models",
+ type,
databaseId: SAVED_QUESTIONS_VIRTUAL_DB_ID,
schemaId,
collectionId: table.collection?.id || "root",
@@ -85,7 +84,6 @@ function getNextValue(table: TableSearchResult): DataPickerValue {
}
function DataSearch({
- value,
searchQuery,
availableDataTypes,
onChange,
@@ -94,11 +92,8 @@ function DataSearch({
const { setQuery } = search;
const searchModels: SearchModel[] = useMemo(() => {
- if (!value.type) {
- return availableDataTypes.map(type => DATA_TYPE_SEARCH_MODEL_MAP[type]);
- }
- return [DATA_TYPE_SEARCH_MODEL_MAP[value.type]];
- }, [value.type, availableDataTypes]);
+ return availableDataTypes.map(type => DATA_TYPE_SEARCH_MODEL_MAP[type]);
+ }, [availableDataTypes]);
const onSelect = useCallback(
(table: TableSearchResult) => {
diff --git a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Models.unit.spec.ts b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Models.unit.spec.ts
index 7beed8ee8d3..cffbd953010 100644
--- a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Models.unit.spec.ts
+++ b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Models.unit.spec.ts
@@ -17,6 +17,7 @@ import {
SAMPLE_MODEL,
SAMPLE_MODEL_2,
SAMPLE_MODEL_3,
+ SAMPLE_QUESTION,
} from "./common";
const ROOT_COLLECTION_MODEL_VIRTUAL_SCHEMA_ID = getCollectionVirtualSchemaId(
@@ -189,4 +190,54 @@ describe("DataPicker — picking models", () => {
screen.queryByRole("button", { name: /Back/i }),
).not.toBeInTheDocument();
});
+
+ it("should be able to search for a model", async () => {
+ const { onChange } = await setup({
+ filters: {
+ types: type => type === "models",
+ },
+ });
+
+ userEvent.type(screen.getByRole("textbox"), SAMPLE_MODEL.name);
+ expect(await screen.findByText(SAMPLE_MODEL.name)).toBeInTheDocument();
+ expect(screen.queryByText(SAMPLE_QUESTION.name)).not.toBeInTheDocument();
+
+ userEvent.click(screen.getByText(SAMPLE_MODEL.name));
+ expect(onChange).toHaveBeenLastCalledWith({
+ type: "models",
+ databaseId: SAVED_QUESTIONS_VIRTUAL_DB_ID,
+ schemaId: getCollectionVirtualSchemaId(SAMPLE_COLLECTION, {
+ isDatasets: true,
+ }),
+ collectionId: SAMPLE_MODEL.collection_id,
+ tableIds: [getQuestionVirtualTableId(SAMPLE_MODEL.id)],
+ });
+ });
+
+ it("should be able to search for a model when a question was selected", async () => {
+ const { onChange } = await setup({
+ initialValue: {
+ type: "questions",
+ databaseId: SAVED_QUESTIONS_VIRTUAL_DB_ID,
+ schemaId: getCollectionVirtualSchemaId(SAMPLE_COLLECTION),
+ collectionId: "root",
+ tableIds: [getQuestionVirtualTableId(SAMPLE_QUESTION.id)],
+ },
+ });
+
+ userEvent.type(screen.getByRole("textbox"), "Sample");
+ expect(await screen.findByText(SAMPLE_MODEL.name)).toBeInTheDocument();
+ expect(screen.getByText(SAMPLE_QUESTION.name)).toBeInTheDocument();
+
+ userEvent.click(screen.getByText(SAMPLE_MODEL.name));
+ expect(onChange).toHaveBeenLastCalledWith({
+ type: "models",
+ databaseId: SAVED_QUESTIONS_VIRTUAL_DB_ID,
+ schemaId: getCollectionVirtualSchemaId(SAMPLE_COLLECTION, {
+ isDatasets: true,
+ }),
+ collectionId: SAMPLE_MODEL.collection_id,
+ tableIds: [getQuestionVirtualTableId(SAMPLE_MODEL.id)],
+ });
+ });
});
diff --git a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Questions.unit.spec.ts b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Questions.unit.spec.ts
index a3c8f97aaa3..67b506f1fa6 100644
--- a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Questions.unit.spec.ts
+++ b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Questions.unit.spec.ts
@@ -11,12 +11,13 @@ import {
} from "metabase-lib/metadata/utils/saved-questions";
import {
- setup,
EMPTY_COLLECTION,
SAMPLE_COLLECTION,
+ SAMPLE_MODEL,
SAMPLE_QUESTION,
SAMPLE_QUESTION_2,
SAMPLE_QUESTION_3,
+ setup,
} from "./common";
const ROOT_COLLECTION_QUESTIONS_VIRTUAL_SCHEMA_ID =
@@ -167,4 +168,52 @@ describe("DataPicker — picking questions", () => {
tableIds: [],
});
});
+
+ it("should be able to search for a question", async () => {
+ const { onChange } = await setup({
+ filters: {
+ types: type => type === "questions",
+ },
+ });
+
+ userEvent.type(screen.getByRole("textbox"), SAMPLE_QUESTION.name);
+ expect(await screen.findByText(SAMPLE_QUESTION.name)).toBeInTheDocument();
+ expect(screen.queryByText(SAMPLE_MODEL.name)).not.toBeInTheDocument();
+
+ userEvent.click(screen.getByText(SAMPLE_QUESTION.name));
+ expect(onChange).toHaveBeenLastCalledWith({
+ type: "questions",
+ databaseId: SAVED_QUESTIONS_VIRTUAL_DB_ID,
+ schemaId: getCollectionVirtualSchemaId(SAMPLE_COLLECTION),
+ collectionId: SAMPLE_QUESTION.collection_id,
+ tableIds: [getQuestionVirtualTableId(SAMPLE_QUESTION.id)],
+ });
+ });
+
+ it("should be able to search for a question when a model was selected", async () => {
+ const { onChange } = await setup({
+ initialValue: {
+ type: "questions",
+ databaseId: SAVED_QUESTIONS_VIRTUAL_DB_ID,
+ schemaId: getCollectionVirtualSchemaId(SAMPLE_COLLECTION, {
+ isDatasets: true,
+ }),
+ collectionId: "root",
+ tableIds: [getQuestionVirtualTableId(SAMPLE_MODEL.id)],
+ },
+ });
+
+ userEvent.type(screen.getByRole("textbox"), "Sample");
+ expect(await screen.findByText(SAMPLE_QUESTION.name)).toBeInTheDocument();
+ expect(screen.getByText(SAMPLE_MODEL.name)).toBeInTheDocument();
+
+ userEvent.click(screen.getByText(SAMPLE_QUESTION.name));
+ expect(onChange).toHaveBeenLastCalledWith({
+ type: "questions",
+ databaseId: SAVED_QUESTIONS_VIRTUAL_DB_ID,
+ schemaId: getCollectionVirtualSchemaId(SAMPLE_COLLECTION),
+ collectionId: SAMPLE_QUESTION.collection_id,
+ tableIds: [getQuestionVirtualTableId(SAMPLE_QUESTION.id)],
+ });
+ });
});
diff --git a/frontend/src/metabase/containers/DataPicker/tests/common.tsx b/frontend/src/metabase/containers/DataPicker/tests/common.tsx
index 60e5273f55b..df4730c9c07 100644
--- a/frontend/src/metabase/containers/DataPicker/tests/common.tsx
+++ b/frontend/src/metabase/containers/DataPicker/tests/common.tsx
@@ -1,6 +1,5 @@
/* istanbul ignore file */
import React from "react";
-import fetchMock from "fetch-mock";
import {
renderWithProviders,
@@ -11,13 +10,16 @@ import {
setupCollectionsEndpoints,
setupCollectionVirtualSchemaEndpoints,
setupDatabasesEndpoints,
+ setupSearchEndpoints,
} from "__support__/server-mocks";
+import Input from "metabase/core/components/Input";
import { ROOT_COLLECTION } from "metabase/entities/collections";
import {
createMockCard,
createMockCollection,
+ createMockCollectionItem,
createMockDatabase,
createMockTable,
} from "metabase-types/api/mocks";
@@ -25,6 +27,7 @@ import { createMockSettingsState } from "metabase-types/store/mocks";
import type { DataPickerValue, DataPickerFiltersProp } from "../types";
import useDataPickerValue from "../useDataPickerValue";
+import { useDataPicker } from "../../DataPicker";
import DataPicker from "../DataPickerContainer";
export const SAMPLE_TABLE = createMockTable({
@@ -68,15 +71,18 @@ export const EMPTY_DATABASE = createMockDatabase({
tables: [],
});
+export const SAMPLE_COLLECTION_ID = 1;
+export const EMPTY_COLLECTION_ID = 2;
+
export const SAMPLE_COLLECTION = createMockCollection({
- id: 1,
+ id: SAMPLE_COLLECTION_ID,
name: "Sample Collection",
location: "/",
here: ["card", "dataset"],
});
export const EMPTY_COLLECTION = createMockCollection({
- id: 2,
+ id: EMPTY_COLLECTION_ID,
name: "Empty Collection",
location: "/",
here: [],
@@ -87,33 +93,51 @@ export const SAMPLE_MODEL = createMockCard({
id: 1,
name: "Sample Model",
dataset: true,
+ collection_id: SAMPLE_COLLECTION_ID,
});
export const SAMPLE_MODEL_2 = createMockCard({
id: 2,
name: "Sample Model 2",
dataset: true,
+ collection_id: SAMPLE_COLLECTION_ID,
});
export const SAMPLE_MODEL_3 = createMockCard({
id: 3,
name: "Sample Model 3",
dataset: true,
+ collection_id: SAMPLE_COLLECTION_ID,
});
export const SAMPLE_QUESTION = createMockCard({
id: 4,
name: "Sample Saved Question",
+ collection_id: SAMPLE_COLLECTION_ID,
});
export const SAMPLE_QUESTION_2 = createMockCard({
id: 5,
name: "Sample Saved Question 2",
+ collection_id: SAMPLE_COLLECTION_ID,
});
export const SAMPLE_QUESTION_3 = createMockCard({
id: 6,
name: "Sample Saved Question 3",
+ collection_id: SAMPLE_COLLECTION_ID,
+});
+
+export const SAMPLE_QUESTION_SEARCH_ITEM = createMockCollectionItem({
+ ...SAMPLE_QUESTION,
+ model: "card",
+ collection: SAMPLE_COLLECTION,
+});
+
+export const SAMPLE_MODEL_SEARCH_ITEM = createMockCollectionItem({
+ ...SAMPLE_MODEL,
+ model: "dataset",
+ collection: SAMPLE_COLLECTION,
});
function DataPickerWrapper({
@@ -129,18 +153,28 @@ function DataPickerWrapper({
}) {
const [value, setValue] = useDataPickerValue(initialValue);
return (
- <DataPicker
- value={value}
- filters={filters}
- isMultiSelect={isMultiSelect}
- onChange={(value: DataPickerValue) => {
- setValue(value);
- onChange(value);
- }}
- />
+ <DataPicker.Provider>
+ <DataPickerSearchInput />
+ <DataPicker
+ value={value}
+ filters={filters}
+ isMultiSelect={isMultiSelect}
+ onChange={(value: DataPickerValue) => {
+ setValue(value);
+ onChange(value);
+ }}
+ />
+ </DataPicker.Provider>
);
}
+function DataPickerSearchInput() {
+ const { search } = useDataPicker();
+ const { query, setQuery } = search;
+
+ return <Input value={query} onChange={e => setQuery(e.target.value)} />;
+}
+
interface SetupOpts {
initialValue?: DataPickerValue;
filters?: DataPickerFiltersProp;
@@ -182,16 +216,6 @@ export async function setup({
setupDatabasesEndpoints([], { hasSavedQuestions: false });
}
- fetchMock.get(
- {
- url: "path:/api/search",
- query: { models: "dataset", limit: 1 },
- },
- {
- data: hasModels ? [SAMPLE_MODEL] : [],
- },
- );
-
setupCollectionsEndpoints([SAMPLE_COLLECTION, EMPTY_COLLECTION]);
setupCollectionVirtualSchemaEndpoints(createMockCollection(ROOT_COLLECTION), [
@@ -210,6 +234,15 @@ export async function setup({
setupCollectionVirtualSchemaEndpoints(EMPTY_COLLECTION, []);
+ if (hasModels) {
+ setupSearchEndpoints([
+ SAMPLE_QUESTION_SEARCH_ITEM,
+ SAMPLE_MODEL_SEARCH_ITEM,
+ ]);
+ } else {
+ setupSearchEndpoints([SAMPLE_QUESTION_SEARCH_ITEM]);
+ }
+
const settings = createMockSettingsState({
"enable-nested-queries": hasNestedQueriesEnabled,
});
diff --git a/frontend/src/metabase/query_builder/components/DataSelector/data-search/SearchResults.jsx b/frontend/src/metabase/query_builder/components/DataSelector/data-search/SearchResults.jsx
index 1c7e816791b..2f89d417338 100644
--- a/frontend/src/metabase/query_builder/components/DataSelector/data-search/SearchResults.jsx
+++ b/frontend/src/metabase/query_builder/components/DataSelector/data-search/SearchResults.jsx
@@ -10,8 +10,8 @@ import Search from "metabase/entities/search";
const propTypes = {
databaseId: PropTypes.string,
- searchQuery: PropTypes.string.required,
- onSelect: PropTypes.func.required,
+ searchQuery: PropTypes.string.isRequired,
+ onSelect: PropTypes.func.isRequired,
searchModels: PropTypes.arrayOf(
PropTypes.oneOf(["card", "dataset", "table"]),
),
diff --git a/frontend/test/__support__/server-mocks/search.ts b/frontend/test/__support__/server-mocks/search.ts
index 146d9e9170a..38c859a1d0c 100644
--- a/frontend/test/__support__/server-mocks/search.ts
+++ b/frontend/test/__support__/server-mocks/search.ts
@@ -1,24 +1,22 @@
import fetchMock from "fetch-mock";
-import type { SearchModelType, CollectionItem } from "metabase-types/api";
+import type { CollectionItem } from "metabase-types/api";
-export function setupSearchEndpoints(
- items: CollectionItem[],
- models: SearchModelType[] = [],
-) {
- fetchMock.get("path:/api/search", {
- available_models: [
- "dashboard",
- "card",
- "dataset",
- "collection",
- "table",
- "database",
- ],
- data: items,
- total: items.length,
- models, // this should reflect what is in the query param
- limit: null,
- offset: null,
- table_db_id: null,
+export function setupSearchEndpoints(items: CollectionItem[]) {
+ const availableModels = items.map(({ model }) => model);
+
+ fetchMock.get("path:/api/search", uri => {
+ const url = new URL(uri);
+ const models = url.searchParams.getAll("models");
+ const matchedItems = items.filter(({ model }) => models.includes(model));
+
+ return {
+ data: matchedItems,
+ total: matchedItems.length,
+ models, // this should reflect what is in the query param
+ available_models: availableModels,
+ limit: null,
+ offset: null,
+ table_db_id: null,
+ };
});
}
--
GitLab