From b874ab183555dcd9f03368817ba6f9d17c4d9519 Mon Sep 17 00:00:00 2001 From: Anton Kulyk <kuliks.anton@gmail.com> Date: Wed, 18 Aug 2021 19:41:48 +0300 Subject: [PATCH] Display official collection tooltips (#17453) * Add tooltips to authority level configs * Use tooltips for CollectionAuthorityLevelIcon * Pass tooltips to getCollectionIcon result * Display tooltip in CollectionHeader * Display tooltip in collections sidebar * Display tooltip on the homepage * Display tooltip in ItemPicker * Accept icon props as a TreeNode's icon prop value * Display tooltip in saved question picker * Accept icon props as a SelectListItem's icon prop value * Display tooltip in dashboard's question picker * Add "Belongs to an Official collection" tooltip * Fix prop-types errors * Add tests for CollectionAuthorityLevelIcon * Add tests for isRegularCollection * Fix search results collection icons * Show "Official Collection" label in search results * Don't show tooltips for official collections in search * Fix ItemPicker's prop type * Add basic test for ItemPicker * Test read-only collections are not displayed * Add test IDs to ItemPicker * Refresh Redux store in each ItemPicker's test * Test opening a nested collection * Test default breadcrumbs state * Fix ItemPicker's header test ID * Test ItemPicker read-only items visibility * Add test for onChange prop * Test navigating back from an open collection * Test personal collections are grouped in ItemPicker * Assert number of ItemPicker items * Test getCollectionIcon in EE * Add a workaround for tooltip offsets * Add describe block for CollectionsList tests * Make CollectionsList test shorter * Test collection types on CollectionList * Fix SearchResult's info * Add SearchResult tests * Use isRegularCollection for AuthorityLevelIcon --- .../CollectionAuthorityLevelIcon.jsx | 15 +- .../CollectionAuthorityLevelIcon.unit.spec.js | 78 +++++ .../collections/constants.js | 4 + .../metabase-enterprise/collections/index.js | 6 +- .../metabase-enterprise/collections/utils.js | 6 + .../collections/utils.unit.spec.js | 33 +++ .../CollectionHeader/CollectionHeader.jsx | 8 +- .../collections/components/CollectionIcon.jsx | 4 +- .../CollectionsList/CollectionsList.jsx | 17 +- .../CollectionsList.unit.spec.js | 181 ++++++++---- .../metabase/components/CollectionItem.jsx | 2 +- .../components/select-list/SelectListItem.jsx | 11 +- .../src/metabase/components/tree/TreeNode.jsx | 18 +- .../src/metabase/containers/ItemPicker.jsx | 101 ++++--- .../containers/ItemPicker.unit.spec.js | 272 ++++++++++++++++++ .../dashboard/components/DashboardGrid.jsx | 11 +- .../add-card-sidebar/QuestionPicker.jsx | 13 +- frontend/src/metabase/entities/collections.js | 13 +- frontend/src/metabase/hoc/Tooltipify.jsx | 4 +- .../components/saved-question-picker/utils.js | 18 +- .../questions/components/CollectionBadge.jsx | 11 +- .../search/components/SearchResult.info.js | 1 + .../search/components/SearchResult.jsx | 14 +- .../components/SearchResult.unit.spec.js | 95 ++++++ .../components/Visualization.jsx | 1 + .../getCollectionIcon.unit.spec.js | 68 ++++- 26 files changed, 836 insertions(+), 169 deletions(-) create mode 100644 enterprise/frontend/src/metabase-enterprise/collections/components/CollectionAuthorityLevelIcon.unit.spec.js create mode 100644 enterprise/frontend/src/metabase-enterprise/collections/utils.js create mode 100644 enterprise/frontend/src/metabase-enterprise/collections/utils.unit.spec.js create mode 100644 frontend/src/metabase/containers/ItemPicker.unit.spec.js create mode 100644 frontend/src/metabase/search/components/SearchResult.unit.spec.js diff --git a/enterprise/frontend/src/metabase-enterprise/collections/components/CollectionAuthorityLevelIcon.jsx b/enterprise/frontend/src/metabase-enterprise/collections/components/CollectionAuthorityLevelIcon.jsx index e329816ca93..adfa981181b 100644 --- a/enterprise/frontend/src/metabase-enterprise/collections/components/CollectionAuthorityLevelIcon.jsx +++ b/enterprise/frontend/src/metabase-enterprise/collections/components/CollectionAuthorityLevelIcon.jsx @@ -4,23 +4,30 @@ import PropTypes from "prop-types"; import Icon from "metabase/components/Icon"; import { color } from "metabase/lib/colors"; -import { AUTHORITY_LEVELS, REGULAR_COLLECTION } from "../constants"; +import { AUTHORITY_LEVELS } from "../constants"; +import { isRegularCollection } from "../utils"; const propTypes = { + tooltip: PropTypes.string, collection: PropTypes.shape({ authority_level: PropTypes.oneOf(["official"]), }), }; -export function CollectionAuthorityLevelIcon({ collection, ...iconProps }) { - const level = AUTHORITY_LEVELS[collection.authority_level]; - if (!level || level.type === REGULAR_COLLECTION.type) { +export function CollectionAuthorityLevelIcon({ + collection, + tooltip = "default", + ...iconProps +}) { + if (isRegularCollection(collection)) { return null; } + const level = AUTHORITY_LEVELS[collection.authority_level]; return ( <Icon {...iconProps} name={level.icon} + tooltip={level.tooltips?.[tooltip] || tooltip} style={{ color: color(level.color) }} data-testid={`${level.type}-collection-marker`} /> diff --git a/enterprise/frontend/src/metabase-enterprise/collections/components/CollectionAuthorityLevelIcon.unit.spec.js b/enterprise/frontend/src/metabase-enterprise/collections/components/CollectionAuthorityLevelIcon.unit.spec.js new file mode 100644 index 00000000000..778512bf6ee --- /dev/null +++ b/enterprise/frontend/src/metabase-enterprise/collections/components/CollectionAuthorityLevelIcon.unit.spec.js @@ -0,0 +1,78 @@ +import React from "react"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { CollectionAuthorityLevelIcon } from "./CollectionAuthorityLevelIcon"; + +describe("CollectionAuthorityLevelIcon", () => { + describe("regular collections", () => { + [ + { + name: "collection without authority level", + collection: {}, + }, + { + name: "regular collection", + collection: { + authority_level: null, + }, + }, + ].forEach(({ collection, name }) => { + it(`doesn't render for ${name}`, () => { + render(<CollectionAuthorityLevelIcon collection={collection} />); + expect(screen.queryByLabelText("folder icon")).toBeNull(); + }); + }); + }); + + describe("official collections", () => { + const OFFICIAL_COLLECTION = { + authority_level: "official", + }; + + function renderOfficialCollection({ + collection = OFFICIAL_COLLECTION, + ...props + } = {}) { + render( + <CollectionAuthorityLevelIcon collection={collection} {...props} />, + ); + } + + function queryOfficialIcon() { + return screen.queryByLabelText("badge icon"); + } + + it(`renders correctly`, () => { + renderOfficialCollection(); + expect(queryOfficialIcon()).toBeInTheDocument(); + }); + + it(`displays a tooltip by default`, () => { + renderOfficialCollection(); + userEvent.hover(queryOfficialIcon()); + expect(screen.getByRole("tooltip")).toHaveTextContent( + "Official collection", + ); + }); + + it(`can display different tooltip`, () => { + renderOfficialCollection({ tooltip: "belonging" }); + userEvent.hover(queryOfficialIcon()); + expect(screen.getByRole("tooltip")).toHaveTextContent( + "Belongs to an Official collection", + ); + }); + + it(`can display custom tooltip text`, () => { + renderOfficialCollection({ tooltip: "Hello" }); + userEvent.hover(queryOfficialIcon()); + expect(screen.getByRole("tooltip")).toHaveTextContent("Hello"); + }); + + it(`can hide tooltip`, () => { + renderOfficialCollection({ tooltip: null }); + userEvent.hover(queryOfficialIcon()); + expect(screen.queryByLabelText("tooltip")).toBeNull(); + }); + }); +}); diff --git a/enterprise/frontend/src/metabase-enterprise/collections/constants.js b/enterprise/frontend/src/metabase-enterprise/collections/constants.js index 6bff7e651b5..d511493f384 100644 --- a/enterprise/frontend/src/metabase-enterprise/collections/constants.js +++ b/enterprise/frontend/src/metabase-enterprise/collections/constants.js @@ -11,6 +11,10 @@ export const OFFICIAL_COLLECTION = { name: t`Official`, icon: "badge", color: "saturated-yellow", + tooltips: { + default: t`Official collection`, + belonging: t`Belongs to an Official collection`, + }, }; export const AUTHORITY_LEVELS = { diff --git a/enterprise/frontend/src/metabase-enterprise/collections/index.js b/enterprise/frontend/src/metabase-enterprise/collections/index.js index 790eaadab57..4d9a2a0a166 100644 --- a/enterprise/frontend/src/metabase-enterprise/collections/index.js +++ b/enterprise/frontend/src/metabase-enterprise/collections/index.js @@ -11,11 +11,7 @@ import { REGULAR_COLLECTION, OFFICIAL_COLLECTION, } from "./constants"; - -function isRegularCollection({ authority_level }) { - // Root, personal collections don't have `authority_level` - return !authority_level || authority_level === REGULAR_COLLECTION.type; -} +import { isRegularCollection } from "./utils"; PLUGIN_COLLECTIONS.isRegularCollection = isRegularCollection; diff --git a/enterprise/frontend/src/metabase-enterprise/collections/utils.js b/enterprise/frontend/src/metabase-enterprise/collections/utils.js new file mode 100644 index 00000000000..2b880fa8eab --- /dev/null +++ b/enterprise/frontend/src/metabase-enterprise/collections/utils.js @@ -0,0 +1,6 @@ +import { REGULAR_COLLECTION } from "./constants"; + +export function isRegularCollection({ authority_level }) { + // Root, personal collections don't have `authority_level` + return !authority_level || authority_level === REGULAR_COLLECTION.type; +} diff --git a/enterprise/frontend/src/metabase-enterprise/collections/utils.unit.spec.js b/enterprise/frontend/src/metabase-enterprise/collections/utils.unit.spec.js new file mode 100644 index 00000000000..4b9b5732836 --- /dev/null +++ b/enterprise/frontend/src/metabase-enterprise/collections/utils.unit.spec.js @@ -0,0 +1,33 @@ +import { isRegularCollection } from "./utils"; + +describe("Collections plugin utils", () => { + const COLLECTION = { + NO_AUTHORITY_LEVEL: { + id: "root", + name: "Our analytics", + }, + REGULAR: { + authority_level: null, + }, + OFFICIAL: { + authority_level: "official", + }, + }; + + describe("isRegularCollection", () => { + it("returns 'true' if collection is missing an authority level", () => { + const collection = COLLECTION.NO_AUTHORITY_LEVEL; + expect(isRegularCollection(collection)).toBe(true); + }); + + it("returns 'true' for regular collections", () => { + const collection = COLLECTION.REGULAR; + expect(isRegularCollection(collection)).toBe(true); + }); + + it("returns 'false' for official collections", () => { + const collection = COLLECTION.OFFICIAL; + expect(isRegularCollection(collection)).toBe(false); + }); + }); +}); diff --git a/frontend/src/metabase/collections/components/CollectionHeader/CollectionHeader.jsx b/frontend/src/metabase/collections/components/CollectionHeader/CollectionHeader.jsx index fc6dfd63d16..c2ffa584e19 100644 --- a/frontend/src/metabase/collections/components/CollectionHeader/CollectionHeader.jsx +++ b/frontend/src/metabase/collections/components/CollectionHeader/CollectionHeader.jsx @@ -20,13 +20,15 @@ import { ToggleMobileSidebarIcon, } from "./CollectionHeader.styled"; -const { CollectionAuthorityLevelIcon } = PLUGIN_COLLECTION_COMPONENTS; - function Title({ collection, handleToggleMobileSidebar }) { return ( <Flex align="center"> <ToggleMobileSidebarIcon onClick={handleToggleMobileSidebar} /> - <CollectionAuthorityLevelIcon collection={collection} mr={1} size={24} /> + <PLUGIN_COLLECTION_COMPONENTS.CollectionAuthorityLevelIcon + collection={collection} + mr={1} + size={24} + /> <PageHeading className="text-wrap">{collection.name}</PageHeading> {collection.description && ( <Tooltip tooltip={collection.description}> diff --git a/frontend/src/metabase/collections/components/CollectionIcon.jsx b/frontend/src/metabase/collections/components/CollectionIcon.jsx index 4de52d3b276..c2d4788212f 100644 --- a/frontend/src/metabase/collections/components/CollectionIcon.jsx +++ b/frontend/src/metabase/collections/components/CollectionIcon.jsx @@ -11,8 +11,8 @@ const propTypes = { }; export function CollectionIcon({ collection, ...props }) { - const { name, color } = getCollectionIcon(collection); - return <Icon name={name} color={color} {...props} />; + const icon = getCollectionIcon(collection); + return <Icon {...icon} {...props} />; } CollectionIcon.propTypes = propTypes; diff --git a/frontend/src/metabase/collections/containers/CollectionSidebar/Collections/CollectionsList/CollectionsList.jsx b/frontend/src/metabase/collections/containers/CollectionSidebar/Collections/CollectionsList/CollectionsList.jsx index d47bb87e5d8..f2bcf7d9df4 100644 --- a/frontend/src/metabase/collections/containers/CollectionSidebar/Collections/CollectionsList/CollectionsList.jsx +++ b/frontend/src/metabase/collections/containers/CollectionSidebar/Collections/CollectionsList/CollectionsList.jsx @@ -17,7 +17,7 @@ import CollectionDropTarget from "metabase/containers/dnd/CollectionDropTarget"; import { PLUGIN_COLLECTIONS } from "metabase/plugins"; -const { isRegularCollection } = PLUGIN_COLLECTIONS; +const IRREGULAR_COLLECTION_ICON_SIZE = 14; function ToggleChildCollectionButton({ action, collectionId, isOpen }) { const iconName = isOpen ? "chevrondown" : "chevronright"; @@ -35,12 +35,17 @@ function ToggleChildCollectionButton({ action, collectionId, isOpen }) { ); } -function Label({ action, collection, initialIcon, isOpen }) { +function Label({ action, depth, collection, isOpen }) { const { children, id, name } = collection; + const isRegular = PLUGIN_COLLECTIONS.isRegularCollection(collection); const hasChildren = Array.isArray(children) && children.some(child => !child.archived); + // Workaround: collection icons on the first tree level incorrect offset out of the box + const targetOffsetX = + !isRegular && depth === 1 ? IRREGULAR_COLLECTION_ICON_SIZE : 0; + return ( <LabelContainer> {hasChildren && ( @@ -51,7 +56,10 @@ function Label({ action, collection, initialIcon, isOpen }) { /> )} - <CollectionListIcon collection={collection} /> + <CollectionListIcon + collection={collection} + targetOffsetX={targetOffsetX} + /> {name} </LabelContainer> ); @@ -78,7 +86,7 @@ function Collection({ {({ highlighted, hovered }) => { const url = Urls.collection(collection); const selected = id === currentCollection; - const dimmedIcon = isRegularCollection(collection); + const dimmedIcon = PLUGIN_COLLECTIONS.isRegularCollection(collection); // when we click on a link, if there are children, // expand to show sub collections @@ -103,6 +111,7 @@ function Collection({ collection={collection} initialIcon={initialIcon} isOpen={isOpen} + depth={depth} /> </CollectionLink> ); diff --git a/frontend/src/metabase/collections/containers/CollectionSidebar/Collections/CollectionsList/CollectionsList.unit.spec.js b/frontend/src/metabase/collections/containers/CollectionSidebar/Collections/CollectionsList/CollectionsList.unit.spec.js index 6c595172ee1..4940cf72f37 100644 --- a/frontend/src/metabase/collections/containers/CollectionSidebar/Collections/CollectionsList/CollectionsList.unit.spec.js +++ b/frontend/src/metabase/collections/containers/CollectionSidebar/Collections/CollectionsList/CollectionsList.unit.spec.js @@ -1,72 +1,131 @@ import React from "react"; -import { render, screen, fireEvent } from "@testing-library/react"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import { DragDropContextProvider } from "react-dnd"; import HTML5Backend from "react-dnd-html5-backend"; +import { PLUGIN_COLLECTIONS } from "metabase/plugins"; + import CollectionsList from "./CollectionsList"; -const filter = () => true; +describe("CollectionsList", () => { + function setup({ collections = [], openCollections = [], ...props } = {}) { + render( + <DragDropContextProvider backend={HTML5Backend}> + <CollectionsList + collections={collections} + openCollections={openCollections} + filter={() => true} + {...props} + /> + </DragDropContextProvider>, + ); + } + + function collection({ + id, + name = "Collection name", + authority_level = null, + location = "/", + children = [], + archived = false, + } = {}) { + return { + id, + name, + authority_level, + location, + children, + archived, + }; + } + + it("renders a basic collection", () => { + setup({ + collections: [collection({ id: 1, name: "Collection name" })], + }); + + expect(screen.queryByText("Collection name")).toBeVisible(); + }); -const openCollections = []; + it("opens child collection when user clicks on chevron button", () => { + const onOpen = jest.fn(); + setup({ + collections: [ + collection({ + id: 1, + name: "Parent collection name", + children: [ + collection({ + id: 2, + name: "Child collection name", + location: "/2/", + }), + ], + }), + ], + onOpen, + }); -it("renders a basic collection", () => { - const collections = [ - { - archived: false, - children: [], + userEvent.click(screen.getByLabelText("chevronright icon")); + + expect(onOpen).toHaveBeenCalled(); + }); + + describe("Collection types", () => { + const regularCollection = collection({ id: 1, authority_level: null }); + const officialCollection = collection({ id: 1, - location: "/", - name: "Collection name", - }, - ]; - - render( - <DragDropContextProvider backend={HTML5Backend}> - <CollectionsList - collections={collections} - filter={filter} - openCollections={openCollections} - /> - </DragDropContextProvider>, - ); - - screen.getByText("Collection name"); -}); + authority_level: "official", + }); + + describe("OSS", () => { + it("displays folder icon for regular collections", () => { + setup({ collections: [regularCollection] }); + expect(screen.queryByLabelText("folder icon")).toBeInTheDocument(); + expect(screen.queryByLabelText("badge icon")).toBeNull(); + }); + + it("displays folder icon for official collections", () => { + setup({ collections: [officialCollection] }); + expect(screen.queryByLabelText("folder icon")).toBeInTheDocument(); + expect(screen.queryByLabelText("badge icon")).toBeNull(); + }); + }); + + describe("EE", () => { + const ORIGINAL_COLLECTIONS_PLUGIN = { + ...PLUGIN_COLLECTIONS, + }; + + beforeAll(() => { + PLUGIN_COLLECTIONS.isRegularCollection = c => !c.authority_level; + PLUGIN_COLLECTIONS.AUTHORITY_LEVEL = { + ...ORIGINAL_COLLECTIONS_PLUGIN, + official: { + icon: "badge", + }, + }; + }); + + afterAll(() => { + PLUGIN_COLLECTIONS.isRegularCollection = + ORIGINAL_COLLECTIONS_PLUGIN.isRegularCollection; + PLUGIN_COLLECTIONS.AUTHORITY_LEVEL = + ORIGINAL_COLLECTIONS_PLUGIN.AUTHORITY_LEVEL; + }); + + it("displays folder icon for regular collections", () => { + setup({ collections: [regularCollection] }); + expect(screen.queryByLabelText("folder icon")).toBeInTheDocument(); + expect(screen.queryByLabelText("badge icon")).toBeNull(); + }); -it("opens child collection when user clicks on chevron button", () => { - const parentCollection = { - archived: false, - children: [], - id: 1, - location: "/", - name: "Parent collection name", - }; - - const childCollection = { - archived: false, - children: [], - id: 2, - location: "/2/", - name: "Child collection name", - }; - - parentCollection.children = [childCollection]; - - const onOpen = jest.fn(); - - render( - <DragDropContextProvider backend={HTML5Backend}> - <CollectionsList - collections={[parentCollection]} - filter={filter} - onOpen={onOpen} - openCollections={openCollections} - /> - </DragDropContextProvider>, - ); - - const chevronButton = screen.getByLabelText("chevronright icon"); - fireEvent.click(chevronButton); - - expect(onOpen).toHaveBeenCalled(); + it("displays badge icon for official collections", () => { + setup({ collections: [officialCollection] }); + expect(screen.queryByLabelText("folder icon")).toBeNull(); + expect(screen.queryByLabelText("badge icon")).toBeInTheDocument(); + }); + }); + }); }); diff --git a/frontend/src/metabase/components/CollectionItem.jsx b/frontend/src/metabase/components/CollectionItem.jsx index e23793c191f..4581cbd4a27 100644 --- a/frontend/src/metabase/components/CollectionItem.jsx +++ b/frontend/src/metabase/components/CollectionItem.jsx @@ -25,7 +25,7 @@ const CollectionItem = ({ collection, event }) => { <Card hoverable> <CardContent> <IconContainer color={icon.color}> - <CollectionIcon name={icon.name} /> + <CollectionIcon name={icon.name} tooltip={icon.tooltip} /> </IconContainer> <h4 className="overflow-hidden"> <Ellipsified>{collection.name}</Ellipsified> diff --git a/frontend/src/metabase/components/select-list/SelectListItem.jsx b/frontend/src/metabase/components/select-list/SelectListItem.jsx index 0d12b5ffe61..98472d39da8 100644 --- a/frontend/src/metabase/components/select-list/SelectListItem.jsx +++ b/frontend/src/metabase/components/select-list/SelectListItem.jsx @@ -2,13 +2,16 @@ import React from "react"; import PropTypes from "prop-types"; import _ from "underscore"; -import { ItemRoot, ItemIcon, ItemTitle } from "./SelectListItem.styled"; +import { iconPropTypes } from "metabase/components/Icon"; import { useScrollOnMount } from "metabase/hooks/use-scroll-on-mount"; +import { ItemRoot, ItemIcon, ItemTitle } from "./SelectListItem.styled"; + const propTypes = { id: PropTypes.string.isRequired, name: PropTypes.string.isRequired, - icon: PropTypes.string.isRequired, + icon: PropTypes.oneOfType([PropTypes.string, PropTypes.shape(iconPropTypes)]) + .isRequired, iconColor: PropTypes.string, onSelect: PropTypes.func.isRequired, isSelected: PropTypes.bool, @@ -26,7 +29,6 @@ export function SelectListItem({ id, name, icon, - iconColor = "brand", onSelect, isSelected = false, rightIcon, @@ -35,6 +37,7 @@ export function SelectListItem({ }) { const ref = useScrollOnMount(); + const iconProps = _.isObject(icon) ? icon : { name: icon }; const rightIconProps = _.isObject(rightIcon) ? rightIcon : { name: rightIcon }; @@ -50,7 +53,7 @@ export function SelectListItem({ onKeyDown={e => e.key === "Enter" && onSelect(id)} className={className} > - <ItemIcon name={icon} color={iconColor} /> + <ItemIcon color="brand" {...iconProps} /> <ItemTitle>{name}</ItemTitle> {rightIconProps.name && <ItemIcon {...rightIconProps} />} </ItemRoot> diff --git a/frontend/src/metabase/components/tree/TreeNode.jsx b/frontend/src/metabase/components/tree/TreeNode.jsx index 315da7e64d8..c1acd2457cc 100644 --- a/frontend/src/metabase/components/tree/TreeNode.jsx +++ b/frontend/src/metabase/components/tree/TreeNode.jsx @@ -1,5 +1,9 @@ import React from "react"; import PropTypes from "prop-types"; +import _ from "underscore"; + +import Icon, { iconPropTypes } from "metabase/components/Icon"; + import { TreeNodeRoot, ExpandToggleButton, @@ -9,8 +13,6 @@ import { RightArrowContainer, } from "./TreeNode.styled"; -import Icon from "metabase/components/Icon"; - const propTypes = { isExpanded: PropTypes.bool.isRequired, isSelected: PropTypes.bool.isRequired, @@ -20,8 +22,10 @@ const propTypes = { depth: PropTypes.number.isRequired, item: PropTypes.shape({ name: PropTypes.string.isRequired, - icon: PropTypes.string, - iconColor: PropTypes.string, + icon: PropTypes.oneOfType([ + PropTypes.string, + PropTypes.shape(iconPropTypes), + ]), hasRightArrow: PropTypes.string, id: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), }).isRequired, @@ -43,7 +47,9 @@ export const TreeNode = React.memo( }, ref, ) { - const { name, icon, iconColor, hasRightArrow, id } = item; + const { name, icon, hasRightArrow, id } = item; + + const iconProps = _.isObject(icon) ? icon : { name: icon }; const handleSelect = () => { onSelect(item); @@ -81,7 +87,7 @@ export const TreeNode = React.memo( {icon && ( <IconContainer variant={variant}> - <Icon name={icon} color={iconColor} /> + <Icon {...iconProps} /> </IconContainer> )} <NameContainer>{name}</NameContainer> diff --git a/frontend/src/metabase/containers/ItemPicker.jsx b/frontend/src/metabase/containers/ItemPicker.jsx index 611e5dd313d..7d8264c6dd2 100644 --- a/frontend/src/metabase/containers/ItemPicker.jsx +++ b/frontend/src/metabase/containers/ItemPicker.jsx @@ -4,6 +4,7 @@ import PropTypes from "prop-types"; import cx from "classnames"; import { t } from "ttag"; +import _ from "underscore"; import { Flex, Box } from "grid-styled"; import Icon from "metabase/components/Icon"; import Breadcrumbs from "metabase/components/Breadcrumbs"; @@ -52,7 +53,7 @@ export default class ItemPicker extends React.Component { // number = non-root collection id value: PropTypes.number, types: PropTypes.array, - showSearch: PropTypes.boolean, + showSearch: PropTypes.bool, }; // returns a list of "crumbs" starting with the "root" collection @@ -153,7 +154,12 @@ export default class ItemPicker extends React.Component { <LoadingAndErrorWrapper loading={!collectionsById} className="scroll-y"> <Box style={style} className={cx(className, "scroll-y")}> {searchMode ? ( - <Box pb={1} mb={2} className="border-bottom flex align-center"> + <Box + pb={1} + mb={2} + className="border-bottom flex align-center" + data-testid="item-picker-header" + > <input type="search" className="input rounded flex-full" @@ -174,7 +180,12 @@ export default class ItemPicker extends React.Component { /> </Box> ) : ( - <Box pb={1} mb={2} className="border-bottom flex align-center"> + <Box + pb={1} + mb={2} + className="border-bottom flex align-center" + data-testid="item-picker-header" + > <Breadcrumbs crumbs={crumbs} /> {showSearch && ( <Icon @@ -185,7 +196,7 @@ export default class ItemPicker extends React.Component { )} </Box> )} - <Box className="scroll-y"> + <Box className="scroll-y" data-testid="item-picker-list"> {!searchString ? allCollections.map(collection => { const hasChildren = @@ -209,7 +220,7 @@ export default class ItemPicker extends React.Component { item={collection} name={collection.name} color={color(icon.color) || COLLECTION_ICON_COLOR} - icon={icon.name} + icon={icon} selected={canSelect && isSelected(collection)} canSelect={canSelect} hasChildren={hasChildren} @@ -286,41 +297,45 @@ const Item = ({ hasChildren, onChange, onChangeParentId, -}) => ( - <Box - mt={1} - p={1} - onClick={ - canSelect - ? () => onChange(item) - : hasChildren - ? () => onChangeParentId(item.id) - : null - } - className={cx("rounded", { - "bg-brand text-white": selected, - "bg-brand-hover text-white-hover cursor-pointer": - canSelect || hasChildren, - })} - > - <Flex align="center"> - <Icon name={icon} color={selected ? "white" : color} size={22} /> - <h4 className="mx1">{name}</h4> - {hasChildren && ( - <Icon - name="chevronright" - className={cx( - "p1 ml-auto circular text-light border-grey-2 bordered bg-white-hover cursor-pointer", - { - "bg-brand-hover": !canSelect, - }, - )} - onClick={e => { - e.stopPropagation(); - onChangeParentId(item.id); - }} - /> - )} - </Flex> - </Box> -); +}) => { + const iconProps = _.isObject(icon) ? icon : { name: icon }; + return ( + <Box + mt={1} + p={1} + onClick={ + canSelect + ? () => onChange(item) + : hasChildren + ? () => onChangeParentId(item.id) + : null + } + className={cx("rounded", { + "bg-brand text-white": selected, + "bg-brand-hover text-white-hover cursor-pointer": + canSelect || hasChildren, + })} + data-testid="item-picker-item" + > + <Flex align="center"> + <Icon size={22} {...iconProps} color={selected ? "white" : color} /> + <h4 className="mx1">{name}</h4> + {hasChildren && ( + <Icon + name="chevronright" + className={cx( + "p1 ml-auto circular text-light border-grey-2 bordered bg-white-hover cursor-pointer", + { + "bg-brand-hover": !canSelect, + }, + )} + onClick={e => { + e.stopPropagation(); + onChangeParentId(item.id); + }} + /> + )} + </Flex> + </Box> + ); +}; diff --git a/frontend/src/metabase/containers/ItemPicker.unit.spec.js b/frontend/src/metabase/containers/ItemPicker.unit.spec.js new file mode 100644 index 00000000000..2dbd7a4bb26 --- /dev/null +++ b/frontend/src/metabase/containers/ItemPicker.unit.spec.js @@ -0,0 +1,272 @@ +import React from "react"; +import { Provider } from "react-redux"; +import { + render, + screen, + waitForElementToBeRemoved, + within, +} from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import xhrMock from "xhr-mock"; +import { getStore } from "__support__/entities-store"; +import ItemPicker from "./ItemPicker"; + +function collection({ + id, + name, + location = "/", + personal_owner_id = null, + can_write = true, +}) { + return { + id, + name, + location, + personal_owner_id, + can_write, + archived: false, + }; +} + +function dashboard({ id, name, collection_id = null }) { + return { + id, + name, + collection_id, + archived: false, + model: "dashboard", + }; +} + +const CURRENT_USER = { + id: 1, + personal_collection_id: 100, + is_superuser: true, +}; + +const COLLECTION = { + ROOT: collection({ id: "root", name: "Our analytics", location: null }), + PERSONAL: collection({ + id: CURRENT_USER.personal_collection_id, + name: "My personal collection", + personal_owner_id: CURRENT_USER.id, + }), + REGULAR: collection({ id: 1, name: "Regular collection" }), + READ_ONLY: collection({ + id: 2, + name: "Read only collection", + can_write: false, + }), +}; + +COLLECTION.REGULAR_CHILD = collection({ + id: 3, + name: "Regular collection's child", + location: `/${COLLECTION.REGULAR.id}/`, +}); + +const COLLECTION_READ_ONLY_CHILD_WRITABLE = collection({ + id: 4, + name: "Read-only collection's child (writable)", + location: `/${COLLECTION.READ_ONLY.id}/`, +}); + +const COLLECTION_OTHER_USERS = collection({ + id: 5, + name: "John Lennon's personal collection", + personal_owner_id: CURRENT_USER.id + 1, +}); + +const DASHBOARD = { + REGULAR: dashboard({ id: 1, name: "Regular dashboard" }), + REGULAR_CHILD: dashboard({ + id: 2, + name: "Regular dashboard (nested)", + collection_id: COLLECTION.REGULAR.id, + }), +}; + +function mockCollectionEndpoint({ extraCollections = [] } = {}) { + const collections = [...Object.values(COLLECTION), ...extraCollections]; + xhrMock.get("/api/collection", { + body: JSON.stringify(collections), + }); +} + +function mockCollectionItemsEndpoint() { + xhrMock.get(/\/api\/collection\/(root|[1-9]\d*)\/items.*/, (req, res) => { + const collectionIdParam = req.url().path.split("/")[3]; + const collectionId = + collectionIdParam === "root" ? null : parseInt(collectionIdParam, 10); + const dashboards = Object.values(DASHBOARD).filter( + dashboard => dashboard.collection_id === collectionId, + ); + return res.status(200).body( + JSON.stringify({ + total: dashboards.length, + data: dashboards, + }), + ); + }); + + xhrMock.get("/api/collection/root/items", (req, res) => { + const dashboards = Object.values(DASHBOARD).filter( + dashboard => dashboard.collection_id === null, + ); + const collections = Object.values(COLLECTION).filter( + collection => collection.location !== "/", + ); + const data = [...dashboards, ...collections]; + return res.status(200).body( + JSON.stringify({ + total: data.length, + data, + }), + ); + }); +} + +async function setup({ + models = ["dashboard"], + extraCollections = [], + ...props +} = {}) { + mockCollectionEndpoint({ extraCollections }); + mockCollectionItemsEndpoint(); + + const onChange = jest.fn(); + + const store = getStore({ + currentUser: () => CURRENT_USER, + }); + + render( + <Provider store={store}> + <ItemPicker models={models} onChange={onChange} {...props} /> + </Provider>, + ); + + await waitForElementToBeRemoved(() => screen.queryByText("Loading...")); + + return { onChange }; +} + +function getItemPickerHeader() { + return within(screen.getByTestId("item-picker-header")); +} + +function getItemPickerList() { + return within(screen.getByTestId("item-picker-list")); +} + +function queryListItem(itemName) { + const node = getItemPickerList() + .queryByText(itemName) + .closest("[data-testid=item-picker-item]"); + return within(node); +} + +async function openCollection(itemName) { + const collectionNode = queryListItem(itemName); + userEvent.click(collectionNode.getByLabelText("chevronright icon")); + await waitForElementToBeRemoved(() => screen.queryByText("Loading...")); +} + +describe("ItemPicker", () => { + beforeEach(() => { + xhrMock.setup(); + }); + + afterEach(() => { + xhrMock.teardown(); + }); + + it("displays items from the root collection by default", async () => { + await setup(); + + // Breadcrumbs + expect( + getItemPickerHeader().queryByText(/Our analytics/i), + ).toBeInTheDocument(); + + // Content + expect(screen.queryByText(DASHBOARD.REGULAR.name)).toBeInTheDocument(); + expect(screen.queryByText(COLLECTION.REGULAR.name)).toBeInTheDocument(); + expect(screen.queryByText(COLLECTION.PERSONAL.name)).toBeInTheDocument(); + expect(screen.queryAllByTestId("item-picker-item")).toHaveLength(3); + }); + + it("does not display read-only collections", async () => { + await setup(); + expect(screen.queryByText(COLLECTION.READ_ONLY.name)).toBeNull(); + }); + + it("displays read-only collections if they have writable children", async () => { + await setup({ extraCollections: [COLLECTION_READ_ONLY_CHILD_WRITABLE] }); + expect(screen.queryByText(COLLECTION.READ_ONLY.name)).toBeInTheDocument(); + }); + + it("can open nested collection", async () => { + await setup(); + + await openCollection(COLLECTION.REGULAR.name); + + const header = getItemPickerHeader(); + const list = getItemPickerList(); + + // Breadcrumbs + expect(header.queryByText(/Our analytics/i)).toBeInTheDocument(); + expect(header.queryByText(COLLECTION.REGULAR.name)).toBeInTheDocument(); + + // Content + expect(list.queryByText(COLLECTION.REGULAR_CHILD.name)).toBeInTheDocument(); + expect(list.queryByText(DASHBOARD.REGULAR_CHILD.name)).toBeInTheDocument(); + expect(list.queryAllByTestId("item-picker-item")).toHaveLength(2); + }); + + it("can navigate back from a currently open nested collection", async () => { + await setup(); + await openCollection(COLLECTION.REGULAR.name); + let header = getItemPickerHeader(); + + userEvent.click(header.getByText(/Our analytics/i)); + + header = getItemPickerHeader(); + const list = getItemPickerList(); + + expect(header.queryByText(COLLECTION.REGULAR.name)).toBeNull(); + + expect(list.queryByText(DASHBOARD.REGULAR.name)).toBeInTheDocument(); + expect(list.queryByText(COLLECTION.REGULAR.name)).toBeInTheDocument(); + expect(list.queryByText(COLLECTION.PERSONAL.name)).toBeInTheDocument(); + expect(list.queryAllByTestId("item-picker-item")).toHaveLength(3); + }); + + it("calls onChange when selecting an item", async () => { + const { onChange } = await setup(); + + userEvent.click(screen.getByText(DASHBOARD.REGULAR.name)); + + expect(onChange).toHaveBeenCalledWith( + expect.objectContaining(DASHBOARD.REGULAR), + ); + expect(onChange).toHaveBeenCalledTimes(1); + }); + + it("doesn't call onChange if it's not a collection picker", async () => { + const { onChange } = await setup(); + userEvent.click(screen.getByText(COLLECTION.REGULAR.name)); + expect(onChange).not.toHaveBeenCalled(); + }); + + it("groups personal collections into single folder if there are more than one", async () => { + await setup({ extraCollections: [COLLECTION_OTHER_USERS] }); + + userEvent.click(screen.queryByText(/All personal collections/i)); + + const list = getItemPickerList(); + expect(list.queryByText(COLLECTION_OTHER_USERS.name)).toBeInTheDocument(); + expect(list.queryByText(COLLECTION.PERSONAL.name)).toBeInTheDocument(); + expect(list.queryAllByTestId("item-picker-item")).toHaveLength(2); + }); +}); diff --git a/frontend/src/metabase/dashboard/components/DashboardGrid.jsx b/frontend/src/metabase/dashboard/components/DashboardGrid.jsx index 248fb04a91f..594a68360f4 100644 --- a/frontend/src/metabase/dashboard/components/DashboardGrid.jsx +++ b/frontend/src/metabase/dashboard/components/DashboardGrid.jsx @@ -251,7 +251,16 @@ export default class DashboardGrid extends Component { if (isRegularDashboard && !isRegularQuestion) { const authorityLevel = dashCard.collection_authority_level; const opts = PLUGIN_COLLECTIONS.AUTHORITY_LEVEL[authorityLevel]; - return { name: opts.icon, color: color(opts.color), size: 14 }; + const iconSize = 14; + return { + name: opts.icon, + color: color(opts.color), + tooltip: opts.tooltips?.belonging, + size: iconSize, + + // Workaround: headerIcon on cards in a first column have incorrect offset out of the box + targetOffsetX: dashCard.col === 0 ? iconSize : 0, + }; } }; diff --git a/frontend/src/metabase/dashboard/components/add-card-sidebar/QuestionPicker.jsx b/frontend/src/metabase/dashboard/components/add-card-sidebar/QuestionPicker.jsx index 3e48499d4fb..a6c0c7bed50 100644 --- a/frontend/src/metabase/dashboard/components/add-card-sidebar/QuestionPicker.jsx +++ b/frontend/src/metabase/dashboard/components/add-card-sidebar/QuestionPicker.jsx @@ -84,16 +84,19 @@ function QuestionPicker({ <SelectList> {collections.map(collection => { const icon = getCollectionIcon(collection); + const iconColor = isRegularCollection(collection) + ? "text-light" + : icon.color; return ( <SelectList.Item - rightIcon="chevronright" key={collection.id} id={collection.id} name={collection.name} - icon={icon.name} - iconColor={ - isRegularCollection(collection) ? "text-light" : icon.color - } + icon={{ + ...icon, + color: iconColor, + }} + rightIcon="chevronright" onSelect={collectionId => setCurrentCollectionId(collectionId) } diff --git a/frontend/src/metabase/entities/collections.js b/frontend/src/metabase/entities/collections.js index 8268b10d8a3..80e4d3433de 100644 --- a/frontend/src/metabase/entities/collections.js +++ b/frontend/src/metabase/entities/collections.js @@ -83,7 +83,10 @@ const Collections = createEntity({ objectSelectors: { getName: collection => collection && collection.name, getUrl: collection => Urls.collection(collection), - getIcon: getCollectionIcon, + getIcon: (collection, opts) => { + const wrappedCollection = collection.collection; + return getCollectionIcon(wrappedCollection || collection, opts); + }, }, selectors: { @@ -137,7 +140,7 @@ const Collections = createEntity({ export default Collections; -export function getCollectionIcon(collection) { +export function getCollectionIcon(collection, { tooltip = "default" } = {}) { if (collection.id === PERSONAL_COLLECTIONS.id) { return { name: "group" }; } @@ -147,7 +150,11 @@ export function getCollectionIcon(collection) { const authorityLevel = PLUGIN_COLLECTIONS.AUTHORITY_LEVEL[collection.authority_level]; return authorityLevel - ? { name: authorityLevel.icon, color: color(authorityLevel.color) } + ? { + name: authorityLevel.icon, + color: color(authorityLevel.color), + tooltip: authorityLevel.tooltips?.[tooltip], + } : { name: "folder" }; } diff --git a/frontend/src/metabase/hoc/Tooltipify.jsx b/frontend/src/metabase/hoc/Tooltipify.jsx index 89a5fd7f30f..d93a0a334ff 100644 --- a/frontend/src/metabase/hoc/Tooltipify.jsx +++ b/frontend/src/metabase/hoc/Tooltipify.jsx @@ -10,10 +10,10 @@ const Tooltipify = ComposedComponent => (ComposedComponent.displayName || ComposedComponent.name) + "]"; render() { - const { tooltip, ...props } = this.props; + const { tooltip, targetOffsetX, ...props } = this.props; if (tooltip) { return ( - <Tooltip tooltip={tooltip}> + <Tooltip tooltip={tooltip} targetOffsetX={targetOffsetX}> <ComposedComponent {...props} /> </Tooltip> ); diff --git a/frontend/src/metabase/query_builder/components/saved-question-picker/utils.js b/frontend/src/metabase/query_builder/components/saved-question-picker/utils.js index 11490016e00..733ccd79d10 100644 --- a/frontend/src/metabase/query_builder/components/saved-question-picker/utils.js +++ b/frontend/src/metabase/query_builder/components/saved-question-picker/utils.js @@ -4,17 +4,13 @@ export function buildCollectionTree(collections) { if (collections == null) { return []; } - return collections.map(collection => { - const icon = getCollectionIcon(collection); - return { - id: collection.id, - name: collection.name, - schemaName: collection.originalName || collection.name, - icon: icon.name, - iconColor: icon.color, - children: buildCollectionTree(collection.children), - }; - }); + return collections.map(collection => ({ + id: collection.id, + name: collection.name, + schemaName: collection.originalName || collection.name, + icon: getCollectionIcon(collection), + children: buildCollectionTree(collection.children), + })); } export const findCollectionByName = (collections, name) => { diff --git a/frontend/src/metabase/questions/components/CollectionBadge.jsx b/frontend/src/metabase/questions/components/CollectionBadge.jsx index b42a908eb53..17e4b9180ae 100644 --- a/frontend/src/metabase/questions/components/CollectionBadge.jsx +++ b/frontend/src/metabase/questions/components/CollectionBadge.jsx @@ -12,6 +12,15 @@ const propTypes = { className: PropTypes.string, }; +const IRREGULAR_ICON_WIDTH = 14; +const IRREGULAR_ICON_PROPS = { + width: IRREGULAR_ICON_WIDTH, + height: 16, + + // Workaround: if a CollectionBadge icon has a tooltip, the default offset x is incorrect + targetOffsetX: IRREGULAR_ICON_WIDTH, +}; + function CollectionBadge({ collection, analyticsContext, className }) { if (!collection) { return null; @@ -19,7 +28,7 @@ function CollectionBadge({ collection, analyticsContext, className }) { const isRegular = PLUGIN_COLLECTIONS.isRegularCollection(collection); const icon = { ...collection.getIcon(), - ...(isRegular ? { size: 12 } : { width: 14, height: 16 }), + ...(isRegular ? { size: 12 } : IRREGULAR_ICON_PROPS), }; return ( <Badge diff --git a/frontend/src/metabase/search/components/SearchResult.info.js b/frontend/src/metabase/search/components/SearchResult.info.js index 961f0099302..1ecda5888c1 100644 --- a/frontend/src/metabase/search/components/SearchResult.info.js +++ b/frontend/src/metabase/search/components/SearchResult.info.js @@ -14,6 +14,7 @@ const COLLECTION_EXAMPLE = { getIcon: () => ({ name: "folder" }), getUrl: () => DEMO_URL, getCollection: () => {}, + collection: {}, }; const DASHBOARD_EXAMPLE = { diff --git a/frontend/src/metabase/search/components/SearchResult.jsx b/frontend/src/metabase/search/components/SearchResult.jsx index ddb4707ec35..04da1b6e5cf 100644 --- a/frontend/src/metabase/search/components/SearchResult.jsx +++ b/frontend/src/metabase/search/components/SearchResult.jsx @@ -106,14 +106,14 @@ function TableIcon() { function CollectionIcon({ item }) { const iconProps = { ...item.getIcon() }; - const isRegular = PLUGIN_COLLECTIONS.isRegularCollection(item); + const isRegular = PLUGIN_COLLECTIONS.isRegularCollection(item.collection); if (isRegular) { iconProps.size = DEFAULT_ICON_SIZE; } else { iconProps.width = 20; iconProps.height = 24; } - return <Icon {...iconProps} />; + return <Icon {...iconProps} tooltip={null} />; } const ModelIconComponentMap = { @@ -215,13 +215,21 @@ function contextText(context) { }); } +function getCollectionInfoText(collection) { + if (PLUGIN_COLLECTIONS.isRegularCollection(collection)) { + return t`Collection`; + } + const level = PLUGIN_COLLECTIONS.AUTHORITY_LEVEL[collection.authority_level]; + return `${level.name} ${t`Collection`}`; +} + function InfoText({ result }) { const collection = result.getCollection(); switch (result.model) { case "card": return jt`Saved question in ${formatCollection(collection)}`; case "collection": - return t`Collection`; + return getCollectionInfoText(result.collection); case "database": return t`Database`; case "table": diff --git a/frontend/src/metabase/search/components/SearchResult.unit.spec.js b/frontend/src/metabase/search/components/SearchResult.unit.spec.js new file mode 100644 index 00000000000..4ee2148b19f --- /dev/null +++ b/frontend/src/metabase/search/components/SearchResult.unit.spec.js @@ -0,0 +1,95 @@ +/* eslint-disable react/prop-types */ +import React from "react"; +import { render, screen } from "@testing-library/react"; +import { PLUGIN_COLLECTIONS } from "metabase/plugins"; +import SearchResult from "./SearchResult"; + +function collection({ + id = 1, + name = "Marketing", + authority_level = null, + getIcon = () => ({ name: "folder" }), + getUrl = () => `/collection/${id}`, + getCollection = () => {}, +} = {}) { + const collection = { + id, + name, + authority_level, + getIcon, + getUrl, + getCollection, + model: "collection", + }; + collection.collection = collection; + return collection; +} + +describe("SearchResult > Collections", () => { + const regularCollection = collection(); + + describe("OSS", () => { + const officialCollection = collection({ + authority_level: "official", + }); + + it("renders regular collection correctly", () => { + render(<SearchResult result={regularCollection} />); + expect(screen.queryByText(regularCollection.name)).toBeInTheDocument(); + expect(screen.queryByText("Collection")).toBeInTheDocument(); + expect(screen.queryByLabelText("folder icon")).toBeInTheDocument(); + expect(screen.queryByLabelText("badge icon")).toBeNull(); + }); + + it("renders official collections as regular", () => { + render(<SearchResult result={officialCollection} />); + expect(screen.queryByText(regularCollection.name)).toBeInTheDocument(); + expect(screen.queryByText("Collection")).toBeInTheDocument(); + expect(screen.queryByLabelText("folder icon")).toBeInTheDocument(); + expect(screen.queryByLabelText("badge icon")).toBeNull(); + }); + }); + + describe("EE", () => { + const officialCollection = collection({ + authority_level: "official", + getIcon: () => ({ name: "badge" }), + }); + + const ORIGINAL_COLLECTIONS_PLUGIN = { ...PLUGIN_COLLECTIONS }; + + beforeAll(() => { + PLUGIN_COLLECTIONS.isRegularCollection = c => !c.authority_level; + PLUGIN_COLLECTIONS.AUTHORITY_LEVEL = { + ...ORIGINAL_COLLECTIONS_PLUGIN.AUTHORITY_LEVEL, + official: { + name: "Official", + icon: "badge", + }, + }; + }); + + afterAll(() => { + PLUGIN_COLLECTIONS.isRegularCollection = + ORIGINAL_COLLECTIONS_PLUGIN.isRegularCollection; + PLUGIN_COLLECTIONS.AUTHORITY_LEVEL = + ORIGINAL_COLLECTIONS_PLUGIN.AUTHORITY_LEVEL; + }); + + it("renders regular collection correctly", () => { + render(<SearchResult result={regularCollection} />); + expect(screen.queryByText(regularCollection.name)).toBeInTheDocument(); + expect(screen.queryByText("Collection")).toBeInTheDocument(); + expect(screen.queryByLabelText("folder icon")).toBeInTheDocument(); + expect(screen.queryByLabelText("badge icon")).toBeNull(); + }); + + it("renders official collections correctly", () => { + render(<SearchResult result={officialCollection} />); + expect(screen.queryByText(regularCollection.name)).toBeInTheDocument(); + expect(screen.queryByText("Official Collection")).toBeInTheDocument(); + expect(screen.queryByLabelText("badge icon")).toBeInTheDocument(); + expect(screen.queryByLabelText("folder icon")).toBeNull(); + }); + }); +}); diff --git a/frontend/src/metabase/visualizations/components/Visualization.jsx b/frontend/src/metabase/visualizations/components/Visualization.jsx index 4a8cdc1b412..e87baeea71e 100644 --- a/frontend/src/metabase/visualizations/components/Visualization.jsx +++ b/frontend/src/metabase/visualizations/components/Visualization.jsx @@ -68,6 +68,7 @@ type Props = { name: string, color?: string, size?: Number, + tooltip?: string, }, actionButtons: React.Element<any>, diff --git a/frontend/test/metabase/entities/collections/getCollectionIcon.unit.spec.js b/frontend/test/metabase/entities/collections/getCollectionIcon.unit.spec.js index 1e7958846f3..b64d31d0282 100644 --- a/frontend/test/metabase/entities/collections/getCollectionIcon.unit.spec.js +++ b/frontend/test/metabase/entities/collections/getCollectionIcon.unit.spec.js @@ -1,13 +1,15 @@ +import { PLUGIN_COLLECTIONS } from "metabase/plugins"; import { getCollectionIcon, ROOT_COLLECTION, PERSONAL_COLLECTIONS as ALL_PERSONAL_COLLECTIONS_VIRTUAL, } from "metabase/entities/collections"; -// NOTE: getCollectionIcon behaves differently in EE -// e.g. it should not return 'folder' for official collections - describe("getCollectionIcon", () => { + const ORIGINAL_AUTHORITY_LEVELS = { + ...PLUGIN_COLLECTIONS.AUTHORITY_LEVEL, + }; + function collection({ id = 10, personal_owner_id = null, @@ -20,7 +22,7 @@ describe("getCollectionIcon", () => { }; } - const testCases = [ + const commonTestCases = [ { name: "Our analytics", collection: ROOT_COLLECTION, @@ -41,18 +43,64 @@ describe("getCollectionIcon", () => { collection: collection({ personal_owner_id: 4 }), expectedIcon: "person", }, + ]; + + const OFFICIAL_COLLECTION = collection({ authority_level: "official" }); + + const testCasesOSS = [ + ...commonTestCases, { name: "Official collection", - collection: collection({ authority_level: "official" }), + collection: OFFICIAL_COLLECTION, expectedIcon: "folder", }, ]; - testCases.forEach(testCase => { - const { name, collection, expectedIcon } = testCase; - it(`returns '${expectedIcon}' for '${name}'`, () => { - expect(getCollectionIcon(collection)).toMatchObject({ - name: expectedIcon, + const testCasesEE = [ + ...commonTestCases, + { + name: "Official collection", + collection: OFFICIAL_COLLECTION, + expectedIcon: "badge", + }, + ]; + + describe("OSS", () => { + testCasesOSS.forEach(testCase => { + const { name, collection, expectedIcon } = testCase; + it(`returns '${expectedIcon}' for '${name}'`, () => { + expect(getCollectionIcon(collection)).toMatchObject({ + name: expectedIcon, + }); + }); + }); + }); + + describe("EE", () => { + beforeEach(() => { + PLUGIN_COLLECTIONS.AUTHORITY_LEVEL = { + ...ORIGINAL_AUTHORITY_LEVELS, + official: { + type: "official", + icon: "badge", + color: "yellow", + tooltips: { + default: "Official Collection", + }, + }, + }; + }); + + afterEach(() => { + PLUGIN_COLLECTIONS.AUTHORITY_LEVEL = ORIGINAL_AUTHORITY_LEVELS; + }); + + testCasesEE.forEach(testCase => { + const { name, collection, expectedIcon } = testCase; + it(`returns '${expectedIcon}' for '${name}'`, () => { + expect(getCollectionIcon(collection)).toMatchObject({ + name: expectedIcon, + }); }); }); }); -- GitLab