Squashed branch

metabase · Apr 30, 2024 · fffe94d · fffe94d
1 parent 0db1f37
commit fffe94d
Show file tree

Hide file tree

Showing 14 changed files with 221 additions and 265 deletions.
diff --git a/frontend/src/metabase-types/api/collection.ts b/frontend/src/metabase-types/api/collection.ts
@@ -59,7 +59,7 @@ export interface Collection {
 
   location: string | null;
   effective_location?: string; // location path containing only those collections that the user has permission to access
-  effective_ancestors?: Collection[];
+  effective_ancestors?: Pick<Collection, "id" | "name">[];
 
   here?: CollectionContentModel[];
   below?: CollectionContentModel[];
@@ -110,6 +110,7 @@ export interface CollectionItem {
   setPinned?: (isPinned: boolean) => void;
   setCollection?: (collection: Pick<Collection, "id">) => void;
   setCollectionPreview?: (isEnabled: boolean) => void;
+  collection_ancestors?: Pick<Collection, "id" | "name">[];
 }
 
 export interface CollectionListQuery {

diff --git a/frontend/src/metabase-types/api/search.ts b/frontend/src/metabase-types/api/search.ts
@@ -132,6 +132,7 @@ export type SearchRequest = {
   last_edited_by?: UserId[];
   search_native_query?: boolean | null;
   verified?: boolean | null;
+  model_ancestors?: boolean | null;
 
   // this should be in ListCollectionItemsRequest but legacy code expects them here
   collection?: CollectionId;

diff --git a/frontend/src/metabase/admin/performance/components/StrategyForm.tsx b/frontend/src/metabase/admin/performance/components/StrategyForm.tsx
@@ -422,19 +422,9 @@ const MultiplierFieldSubtitle = () => (
   <div>
     {t`To determine how long each cached result should stick around, we take that query's average execution time and multiply that by what you input here. The result is how many seconds the cache should remain valid for.`}{" "}
     <Tooltip
-      events={{
-        hover: true,
-        focus: true,
-        touch: true,
-      }}
+      variant="multiline"
       inline={true}
-      styles={{
-        tooltip: {
-          whiteSpace: "normal",
-        },
-      }}
       label={t`If a query takes on average 120 seconds (2 minutes) to run, and you input 10 for your multiplier, its cache entry will persist for 1,200 seconds (20 minutes).`}
-      maw="20rem"
     >
       <Text tabIndex={0} lh="1" display="inline" c="brand">
         {t`Example`}

diff --git a/frontend/src/metabase/browse/components/BrowseModels.tsx b/frontend/src/metabase/browse/components/BrowseModels.tsx
@@ -5,20 +5,13 @@ import _ from "underscore";
 import NoResults from "assets/img/no_results.svg";
 import { useSearchQuery } from "metabase/api";
 import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper";
+import Search from "metabase/entities/search";
 import { color } from "metabase/lib/colors";
-import { useSelector } from "metabase/lib/redux";
+import { useDispatch } from "metabase/lib/redux";
 import { PLUGIN_CONTENT_VERIFICATION } from "metabase/plugins";
-import { getLocale } from "metabase/setup/selectors";
 import { Box, Flex, Group, Icon, Stack, Title } from "metabase/ui";
-import type { CollectionId } from "metabase-types/api";
 
-import { BROWSE_MODELS_LOCALSTORAGE_KEY } from "../constants";
-import {
-  filterModels,
-  getCollectionViewPreferences,
-  groupModels,
-  type ActualModelFilters,
-} from "../utils";
+import { filterModels, type ActualModelFilters } from "../utils";
 
 import {
   BrowseContainer,
@@ -27,9 +20,8 @@ import {
   BrowseSection,
   CenteredEmptyState,
 } from "./BrowseApp.styled";
-import { ModelGrid } from "./BrowseModels.styled";
 import { ModelExplanationBanner } from "./ModelExplanationBanner";
-import { ModelGroup } from "./ModelGroup";
+import { ModelsTable } from "./ModelsTable";
 
 const { availableModelFilters } = PLUGIN_CONTENT_VERIFICATION;
 
@@ -107,16 +99,13 @@ export const BrowseModelsBody = ({
 }: {
   actualModelFilters: ActualModelFilters;
 }) => {
+  const dispatch = useDispatch();
   const { data, error, isLoading } = useSearchQuery({
     models: ["dataset"],
+    model_ancestors: true,
     filter_items_in_personal_collection: "exclude",
   });
   const unfilteredModels = data?.data;
-  const locale = useSelector(getLocale);
-  const localeCode: string | undefined = locale?.code;
-  const [collectionViewPreferences, setCollectionViewPreferences] = useState(
-    getCollectionViewPreferences,
-  );
 
   const models = useMemo(
     () =>
@@ -128,37 +117,7 @@ export const BrowseModelsBody = ({
     [unfilteredModels, actualModelFilters],
   );
 
-  const handleToggleCollectionExpand = (collectionId: CollectionId) => {
-    const newPreferences = {
-      ...collectionViewPreferences,
-      [collectionId]: {
-        expanded: !(
-          collectionViewPreferences?.[collectionId]?.expanded ?? true
-        ),
-        showAll: !!collectionViewPreferences?.[collectionId]?.showAll,
-      },
-    };
-    setCollectionViewPreferences(newPreferences);
-    localStorage.setItem(
-      BROWSE_MODELS_LOCALSTORAGE_KEY,
-      JSON.stringify(newPreferences),
-    );
-  };
-
-  const handleToggleCollectionShowAll = (collectionId: CollectionId) => {
-    const newPreferences = {
-      ...collectionViewPreferences,
-      [collectionId]: {
-        expanded: collectionViewPreferences?.[collectionId]?.expanded ?? true,
-        showAll: !collectionViewPreferences?.[collectionId]?.showAll,
-      },
-    };
-    setCollectionViewPreferences(newPreferences);
-    localStorage.setItem(
-      BROWSE_MODELS_LOCALSTORAGE_KEY,
-      JSON.stringify(newPreferences),
-    );
-  };
+  const wrappedModels = models.map(model => Search.wrapEntity(model, dispatch));
 
   if (error || isLoading) {
     return (
@@ -170,34 +129,11 @@ export const BrowseModelsBody = ({
     );
   }
 
-  const groupsOfModels = groupModels(models, localeCode);
-
   if (models.length) {
     return (
       <Stack spacing="md" mb="lg">
         <ModelExplanationBanner />
-        <ModelGrid role="grid">
-          {groupsOfModels.map(groupOfModels => {
-            const collectionId = groupOfModels[0].collection.id;
-            return (
-              <ModelGroup
-                expanded={
-                  collectionViewPreferences?.[collectionId]?.expanded ?? true
-                }
-                showAll={!!collectionViewPreferences?.[collectionId]?.showAll}
-                toggleExpanded={() =>
-                  handleToggleCollectionExpand(collectionId)
-                }
-                toggleShowAll={() =>
-                  handleToggleCollectionShowAll(collectionId)
-                }
-                models={groupOfModels}
-                key={`modelgroup-${collectionId}`}
-                localeCode={localeCode}
-              />
-            );
-          })}
-        </ModelGrid>
+        <ModelsTable items={wrappedModels} />
       </Stack>
     );
   }

diff --git a/frontend/src/metabase/browse/components/BrowseModels.unit.spec.tsx b/frontend/src/metabase/browse/components/BrowseModels.unit.spec.tsx
@@ -1,5 +1,3 @@
-import userEvent from "@testing-library/user-event";
-
 import {
   setupSearchEndpoints,
   setupSettingsEndpoints,
@@ -13,11 +11,9 @@ import {
 } from "metabase-types/api/mocks";
 import { createMockSetupState } from "metabase-types/store/mocks";
 
-import { BROWSE_MODELS_LOCALSTORAGE_KEY } from "../constants";
-
 import { BrowseModels } from "./BrowseModels";
 
-const renderBrowseModels = (modelCount: number) => {
+const setup = (modelCount: number) => {
   const models = mockModels.slice(0, modelCount);
   setupSearchEndpoints(models);
   setupSettingsEndpoints([]);
@@ -213,150 +209,29 @@ const mockModels: SearchResult[] = [
 ].map(model => createMockModelResult(model));
 
 describe("BrowseModels", () => {
-  beforeEach(() => {
-    localStorage.clear();
-  });
   it("displays a 'no models' message in the Models tab when no models exist", async () => {
-    renderBrowseModels(0);
+    setup(0);
     expect(await screen.findByText("No models here yet")).toBeInTheDocument();
   });
 
-  it("displays collection groups", async () => {
-    renderBrowseModels(10);
-    expect(await screen.findByText("Alpha")).toBeInTheDocument();
-    expect(await screen.findByText("Beta")).toBeInTheDocument();
-    expect(await screen.findByText("Charlie")).toBeInTheDocument();
-    expect(await screen.findByText("Delta")).toBeInTheDocument();
-  });
-
-  it("displays models in collections by default", () => {
+  it("displays models in collections by default", async () => {
     const modelCount = 22;
-    renderBrowseModels(modelCount);
+    setup(modelCount);
+    expect(await screen.findByRole("table")).toBeInTheDocument();
     expect(screen.queryByText("No models here yet")).not.toBeInTheDocument();
     assertThatModelsExist(0, modelCount - 1);
   });
 
-  it("can collapse collections to hide models within them", async () => {
-    renderBrowseModels(10);
-    await userEvent.click(await screen.findByLabelText("collapse Alpha"));
-    expect(screen.queryByText("Model 0")).not.toBeInTheDocument();
-    expect(screen.queryByText("Model 1")).not.toBeInTheDocument();
-    expect(screen.queryByText("Model 2")).not.toBeInTheDocument();
-
-    await userEvent.click(await screen.findByLabelText("collapse Beta"));
-    expect(screen.queryByText("Model 3")).not.toBeInTheDocument();
-    expect(screen.queryByText("Model 4")).not.toBeInTheDocument();
-    expect(screen.queryByText("Model 5")).not.toBeInTheDocument();
-  });
-
-  it("can expand a collection to see models within it", async () => {
-    renderBrowseModels(10);
-    await userEvent.click(await screen.findByLabelText("collapse Alpha"));
-    expect(screen.queryByText("Model 0")).not.toBeInTheDocument();
-    await userEvent.click(await screen.findByLabelText("expand Alpha"));
-    expect(await screen.findByText("Model 0")).toBeInTheDocument();
-  });
-
   it("displays the Our Analytics collection if it has a model", async () => {
-    renderBrowseModels(25);
-    await screen.findByText("Alpha");
-    await screen.findByText("Our analytics");
+    setup(25);
+    expect(await screen.findByRole("table")).toBeInTheDocument();
+    expect(
+      await screen.findAllByTestId("path-for-collection: Our analytics"),
+    ).toHaveLength(2);
     expect(await screen.findByText("Model 20")).toBeInTheDocument();
     expect(await screen.findByText("Model 21")).toBeInTheDocument();
     expect(await screen.findByText("Model 22")).toBeInTheDocument();
   });
-
-  it("shows the first six models in a collection by default", async () => {
-    renderBrowseModels(9999);
-    expect(await screen.findByText("100 models")).toBeInTheDocument();
-    expect(await screen.findByText("Show all")).toBeInTheDocument();
-    assertThatModelsExist(300, 305);
-  });
-
-  it("can show more than 6 models by clicking 'Show all'", async () => {
-    renderBrowseModels(9999);
-    await screen.findByText("6 of 100");
-    expect(screen.queryByText("Model 350")).not.toBeInTheDocument();
-    await userEvent.click(await screen.findByText("Show all"));
-    assertThatModelsExist(300, 399);
-  });
-
-  it("can show less than all models by clicking 'Show less'", async () => {
-    renderBrowseModels(9999);
-    expect(screen.queryByText("Model 399")).not.toBeInTheDocument();
-    await userEvent.click(await screen.findByText("Show all"));
-    await screen.findByText("Model 301");
-    expect(screen.getByText("Model 399")).toBeInTheDocument();
-    await userEvent.click(await screen.findByText("Show less"));
-    await screen.findByText("Model 301");
-    expect(screen.queryByText("Model 399")).not.toBeInTheDocument();
-  });
-
-  it("persists show-all state when expanding and collapsing collections", async () => {
-    renderBrowseModels(9999);
-    await userEvent.click(screen.getByText("Show all"));
-    expect(await screen.findByText("Model 301")).toBeInTheDocument();
-    expect(screen.getByText("Model 399")).toBeInTheDocument();
-
-    await userEvent.click(screen.getByLabelText("collapse Grande"));
-    expect(screen.queryByText("Model 301")).not.toBeInTheDocument();
-    expect(screen.queryByText("Model 399")).not.toBeInTheDocument();
-
-    await userEvent.click(screen.getByLabelText("expand Grande"));
-    expect(await screen.findByText("Model 301")).toBeInTheDocument();
-    expect(screen.getByText("Model 399")).toBeInTheDocument();
-  });
-
-  describe("local storage", () => {
-    it("persists the expanded state of collections in local storage", async () => {
-      renderBrowseModels(10);
-      await userEvent.click(await screen.findByLabelText("collapse Alpha"));
-      expect(screen.queryByText("Model 0")).not.toBeInTheDocument();
-      expect(localStorage.getItem(BROWSE_MODELS_LOCALSTORAGE_KEY)).toEqual(
-        JSON.stringify({ 99: { expanded: false, showAll: false } }),
-      );
-    });
-
-    it("loads the collapsed state of collections from local storage", async () => {
-      localStorage.setItem(
-        BROWSE_MODELS_LOCALSTORAGE_KEY,
-        JSON.stringify({ 99: { expanded: false, showAll: false } }),
-      );
-      renderBrowseModels(10);
-      expect(screen.queryByText("Model 0")).not.toBeInTheDocument();
-    });
-
-    it("persists the 'show all' state of collections in local storage", async () => {
-      renderBrowseModels(9999);
-      await userEvent.click(await screen.findByText("Show all"));
-      await screen.findByText("Model 399");
-      expect(localStorage.getItem(BROWSE_MODELS_LOCALSTORAGE_KEY)).toEqual(
-        JSON.stringify({ 7: { expanded: true, showAll: true } }),
-      );
-    });
-
-    it("loads the 'show all' state of collections from local storage", async () => {
-      localStorage.setItem(
-        BROWSE_MODELS_LOCALSTORAGE_KEY,
-        JSON.stringify({ 7: { expanded: true, showAll: true } }),
-      );
-      renderBrowseModels(9999);
-      expect(await screen.findByText("Show less")).toBeInTheDocument();
-      assertThatModelsExist(300, 399);
-    });
-
-    it("can deal with invalid local storage data", async () => {
-      localStorage.setItem(BROWSE_MODELS_LOCALSTORAGE_KEY, "{invalid json[[[}");
-      renderBrowseModels(10);
-      expect(await screen.findByText("Model 0")).toBeInTheDocument();
-      await userEvent.click(await screen.findByLabelText("collapse Alpha"));
-      expect(screen.queryByText("Model 0")).not.toBeInTheDocument();
-      // ignores invalid data and persists the new state
-      expect(localStorage.getItem(BROWSE_MODELS_LOCALSTORAGE_KEY)).toEqual(
-        JSON.stringify({ 99: { expanded: false, showAll: false } }),
-      );
-    });
-  });
 });
 
 function assertThatModelsExist(startId: number, endId: number) {

diff --git a/frontend/src/metabase/browse/components/EllipsifiedWithMarkdown.tsx b/frontend/src/metabase/browse/components/EllipsifiedWithMarkdown.tsx
@@ -0,0 +1,18 @@
+import { Ellipsified } from "metabase/core/components/Ellipsified";
+import Markdown from "metabase/core/components/Markdown";
+
+export const EllipsifiedWithMarkdown = ({ children }: { children: string }) => {
+  return (
+    <Ellipsified
+      tooltip={
+        <Markdown disallowHeading unstyleLinks lineClamp={12}>
+          {children}
+        </Markdown>
+      }
+    >
+      <Markdown disallowHeading>
+        {typeof children === "string" ? children.replace(/\s/g, " ") : children}
+      </Markdown>
+    </Ellipsified>
+  );
+};