diff --git a/enterprise/frontend/src/metabase-enterprise/browse/components/BrowseModels.unit.spec.tsx b/enterprise/frontend/src/metabase-enterprise/browse/components/BrowseModels.unit.spec.tsx index 063849f0a9e6065afe52d00e7214370ee07610ca..3dc80a5392d18f8c77f1c3a5f5e9507a642df229 100644 --- a/enterprise/frontend/src/metabase-enterprise/browse/components/BrowseModels.unit.spec.tsx +++ b/enterprise/frontend/src/metabase-enterprise/browse/components/BrowseModels.unit.spec.tsx @@ -5,22 +5,35 @@ import { setupSettingsEndpoints, } from "__support__/server-mocks"; import { mockSettings } from "__support__/settings"; -import { renderWithProviders, screen } from "__support__/ui"; +import { renderWithProviders, screen, within } from "__support__/ui"; import { BrowseModels } from "metabase/browse/components/BrowseModels"; -import { createMockModelResult } from "metabase/browse/test-utils"; -import type { SearchResult } from "metabase-types/api"; +import { + createMockModelResult, + createMockRecentModel, +} from "metabase/browse/test-utils"; +import type { RecentCollectionItem } from "metabase-types/api"; import { createMockCollection, createMockTokenFeatures, } from "metabase-types/api/mocks"; import { createMockSetupState } from "metabase-types/store/mocks"; -const setup = (modelCount: number) => { +const setup = (modelCount: number, recentModelCount = 5) => { const models = mockModels.slice(0, modelCount); + + // Add some instance analytics models to ensure they don't affect the page + models.push(...mockInstanceAnalyticsModels); + + const mockRecentModels = mockModels + .slice(0, recentModelCount) + .map(model => + createMockRecentModel(model as unknown as RecentCollectionItem), + ); + setupRecentViewsEndpoints(mockRecentModels); + setupSearchEndpoints(models); setupSettingsEndpoints([]); setupEnterprisePlugins(); - setupRecentViewsEndpoints([]); return renderWithProviders(<BrowseModels />, { storeInitialState: { setup: createMockSetupState({ @@ -29,6 +42,7 @@ const setup = (modelCount: number) => { settings: mockSettings({ "token-features": createMockTokenFeatures({ official_collections: true, + audit_app: true, }), }), }, @@ -61,7 +75,7 @@ const grandchildOfInstanceAnalyticsCollection = createMockCollection({ ], }); -const mockModels: SearchResult[] = [ +const mockModels = [ createMockModelResult({ id: 0, name: "A normal model", @@ -92,10 +106,102 @@ const mockModels: SearchResult[] = [ }), ]; +const mockInstanceAnalyticsModels = [ + createMockModelResult({ + id: 1000, + name: "Instance analytics model 1", + collection: createMockCollection({ + type: "instance-analytics", + }), + }), + createMockModelResult({ + id: 1001, + name: "Instance analytics model 2", + collection: createMockCollection({ + type: "instance-analytics", + }), + }), + createMockModelResult({ + id: 1002, + name: "Instance analytics model 3", + collection: createMockCollection({ + type: "instance-analytics", + }), + }), + createMockModelResult({ + id: 1003, + name: "Instance analytics model 4", + collection: createMockCollection({ + type: "instance-analytics", + }), + }), + createMockModelResult({ + id: 1004, + name: "Instance analytics model 5", + collection: createMockCollection({ + type: "instance-analytics", + }), + }), + createMockModelResult({ + id: 1005, + name: "Instance analytics model 6", + collection: createMockCollection({ + type: "instance-analytics", + }), + }), + createMockModelResult({ + id: 1006, + name: "Instance analytics model 7", + collection: createMockCollection({ + type: "instance-analytics", + }), + }), + createMockModelResult({ + id: 1007, + name: "Instance analytics model 8", + collection: createMockCollection({ + type: "instance-analytics", + }), + }), + createMockModelResult({ + id: 1008, + name: "Instance analytics model 9", + collection: createMockCollection({ + type: "instance-analytics", + }), + }), + createMockModelResult({ + id: 1009, + name: "Instance analytics model 10", + collection: createMockCollection({ + type: "instance-analytics", + }), + }), +]; + describe("BrowseModels", () => { it("does not display instance analytics collections", async () => { setup(4); - expect(await screen.findByText("A normal model")).toBeInTheDocument(); + const modelsTable = await screen.findByRole("table", { + name: /Table of models/, + }); + expect( + await within(modelsTable).findByText("A normal model"), + ).toBeInTheDocument(); expect(screen.queryByText("instance analytics")).not.toBeInTheDocument(); }); + it("displays recently viewed models when there are enough models", async () => { + setup(9); + const recentModelsGrid = await screen.findByRole("grid", { + name: /Recents/, + }); + expect(recentModelsGrid).toBeInTheDocument(); + }); + it("displays no recently viewed models when there are fewer than 9 models", async () => { + setup(8); + const recentModelsGrid = screen.queryByRole("grid", { + name: /Recents/, + }); + expect(recentModelsGrid).not.toBeInTheDocument(); + }); }); diff --git a/enterprise/frontend/src/metabase-enterprise/collections/utils.unit.spec.ts b/enterprise/frontend/src/metabase-enterprise/collections/utils.unit.spec.tsx similarity index 55% rename from enterprise/frontend/src/metabase-enterprise/collections/utils.unit.spec.ts rename to enterprise/frontend/src/metabase-enterprise/collections/utils.unit.spec.tsx index 7022462d088f16e9cda3aa7d0a260814a806351f..5f3b35e7ee8c793134cecfbd4137eb7096a907b3 100644 --- a/enterprise/frontend/src/metabase-enterprise/collections/utils.unit.spec.ts +++ b/enterprise/frontend/src/metabase-enterprise/collections/utils.unit.spec.tsx @@ -1,6 +1,19 @@ -import { createMockCollection } from "metabase-types/api/mocks"; +import { setupEnterprisePlugins } from "__support__/enterprise"; +import { mockSettings } from "__support__/settings"; +import { renderWithProviders } from "__support__/ui"; +import { createMockModelResult } from "metabase/browse/test-utils"; +import { + createMockCollection, + createMockTokenFeatures, +} from "metabase-types/api/mocks"; +import { createMockState } from "metabase-types/store/mocks"; -import { getCollectionType, getIcon, isRegularCollection } from "./utils"; +import { + filterOutItemsFromInstanceAnalytics, + getCollectionType, + getIcon, + isRegularCollection, +} from "./utils"; describe("Collections plugin utils", () => { const COLLECTION = { @@ -92,4 +105,75 @@ describe("Collections plugin utils", () => { }); }); }); + + describe("filterOutItemsFromInstanceAnalytics", () => { + const state = createMockState({ + settings: mockSettings({ + "token-features": createMockTokenFeatures({ + audit_app: true, + }), + }), + }); + beforeEach(() => { + setupEnterprisePlugins(); + }); + + it("should filter out items directly in an instance analytics collection", () => { + renderWithProviders(<></>, { + storeInitialState: state, + }); + // Ids must be distinct because we cache based on id + const items = [ + createMockModelResult({ + id: 0, + name: "filter this out", + collection: createMockCollection({ + id: 1, + type: "instance-analytics", + }), + }), + createMockModelResult({ + id: 2, + name: "filter this out", + collection: createMockCollection({ + id: 3, + effective_ancestors: [ + createMockCollection({ + id: 4, + type: "instance-analytics", + }), + ], + }), + }), + createMockModelResult({ + id: 5, + name: "filter this out", + collection: createMockCollection({ + id: 6, + effective_ancestors: [ + createMockCollection({ id: 7 }), + createMockCollection({ id: 8, type: "instance-analytics" }), + ], + }), + }), + createMockModelResult({ + id: 9, + name: "keep this", + collection: createMockCollection({ id: 10 }), + }), + ]; + + const result = filterOutItemsFromInstanceAnalytics(items); + expect(result).toHaveLength(1); + expect(result[0].name).toBe("keep this"); + }); + + it("should handle empty input array", () => { + renderWithProviders(<></>, { + storeInitialState: state, + }); + const result = filterOutItemsFromInstanceAnalytics([]); + expect(result).toEqual([]); + }); + }); }); diff --git a/frontend/src/metabase/browse/components/BrowseModels.tsx b/frontend/src/metabase/browse/components/BrowseModels.tsx index a74eb318f4f523d71e6b2c32fa559f53c38a4b82..cdfd609aa1e179d7c76edbe16ec70e9cccc4bd22 100644 --- a/frontend/src/metabase/browse/components/BrowseModels.tsx +++ b/frontend/src/metabase/browse/components/BrowseModels.tsx @@ -37,20 +37,17 @@ export const BrowseModels = () => { const modelsResult = useFetchModels({ model_ancestors: true }); - const { allModels, doVerifiedModelsExist } = useMemo(() => { - const allModels = + const { models, doVerifiedModelsExist } = useMemo(() => { + const unfilteredModels = (modelsResult.data?.data as ModelResult[] | undefined) ?? []; - const doVerifiedModelsExist = allModels.some( + const doVerifiedModelsExist = unfilteredModels.some( model => model.moderated_status === "verified", ); - return { allModels, doVerifiedModelsExist }; + const models = + PLUGIN_COLLECTIONS.filterOutItemsFromInstanceAnalytics(unfilteredModels); + return { models, doVerifiedModelsExist }; }, [modelsResult]); - const models = useMemo( - () => PLUGIN_COLLECTIONS.filterOutItemsFromInstanceAnalytics(allModels), - [allModels], - ); - const { filteredModels } = useMemo(() => { // If no models are verified, don't filter them const filteredModels = doVerifiedModelsExist @@ -74,9 +71,9 @@ export const BrowseModels = () => { ); const recentModels = useMemo(() => { - const cap = getMaxRecentModelCount(allModels.length); + const cap = getMaxRecentModelCount(models.length); return filteredRecentModels.slice(0, cap); - }, [filteredRecentModels, allModels.length]); + }, [filteredRecentModels, models.length]); const isEmpty = !recentModelsResult.isLoading && diff --git a/frontend/src/metabase/browse/components/BrowseModels.unit.spec.tsx b/frontend/src/metabase/browse/components/BrowseModels.unit.spec.tsx index 956d2f64ae8720085f1bf75436b3c00c5e3d8aab..1d059fc2f9d1082022f891c58e95caf15b1e8fc7 100644 --- a/frontend/src/metabase/browse/components/BrowseModels.unit.spec.tsx +++ b/frontend/src/metabase/browse/components/BrowseModels.unit.spec.tsx @@ -332,4 +332,12 @@ describe("BrowseModels", () => { within(recentModelsGrid).queryByText("Model 5"), ).not.toBeInTheDocument(); }); + + it("displays no recently viewed models when there are fewer than 9 models - but instance analytics models do not count", async () => { + setup(8); + const recentModelsGrid = screen.queryByRole("grid", { + name: /Recents/, + }); + expect(recentModelsGrid).not.toBeInTheDocument(); + }); });