Skip to content
Snippets Groups Projects
Unverified Commit 35a5df38 authored by Nemanja Glumac's avatar Nemanja Glumac Committed by GitHub
Browse files

[MS2] Better Onboarding UX: Expanded CSV upload permissions (#48475)

* Use more granular permissions

* Simplify the "upload spreadsheet" button logic

* Simplify the "add database" button logic

* Go back to using the existing `databases` prop

* Conditionally display `UploadInput`

* Refine `canUpload` logic

* Reduce prop drilling

* Make visual logic more "reader-friendly"

* Fix and expand tests

* Pass down the existing prop `hasDataAccess`

* Remove the Metabase "learn" link

* Move sub-components out of the render
parent 000dfe1b
No related branches found
No related tags found
No related merge requests found
......@@ -33,6 +33,7 @@ import type { DashboardState } from "metabase-types/store";
import {
createMockDashboardState,
createMockQueryBuilderState,
createMockSettingsState,
createMockState,
} from "metabase-types/store/mocks";
......@@ -43,10 +44,12 @@ type SetupOpts = {
route?: string;
user?: User | null;
hasDataAccess?: boolean;
hasOwnDatabase?: boolean;
withAdditionalDatabase?: boolean;
isUploadEnabled?: boolean;
openQuestionCard?: Card;
openDashboard?: Dashboard;
models?: ModelResult[];
canCurateRootCollection?: boolean;
};
const PERSONAL_COLLECTION_BASE = createMockCollection({
......@@ -60,38 +63,46 @@ const TEST_COLLECTION = createMockCollection({
name: "Test collection",
});
const SAMPLE_DATABASE = createMockDatabase({
id: 1,
name: "Sample Database",
is_sample: true,
});
const USER_DATABASE = createMockDatabase({
id: 2,
name: "User Database",
is_sample: false,
});
async function setup({
pathname = "/",
route = pathname,
user = createMockUser(),
hasDataAccess = true,
hasOwnDatabase = true,
openDashboard,
openQuestionCard,
models = [],
isUploadEnabled = false,
withAdditionalDatabase = true,
canCurateRootCollection = false,
}: SetupOpts = {}) {
const SAMPLE_DATABASE = createMockDatabase({
id: 1,
name: "Sample Database",
is_sample: true,
can_upload: user?.is_superuser || (isUploadEnabled && hasDataAccess),
});
const USER_DATABASE = createMockDatabase({
id: 2,
name: "User Database",
is_sample: false,
});
const databases = [];
const collections = [TEST_COLLECTION];
if (hasDataAccess) {
databases.push(SAMPLE_DATABASE);
if (hasOwnDatabase) {
if (withAdditionalDatabase) {
databases.push(USER_DATABASE);
}
}
const OUR_ANALYTICS = createMockCollection({
...ROOT_COLLECTION,
can_write: user?.is_superuser || canCurateRootCollection,
});
const collections = [TEST_COLLECTION];
const personalCollection = user
? createMockCollection({
......@@ -105,14 +116,17 @@ async function setup({
collections.push(personalCollection);
}
setupCollectionsEndpoints({ collections });
setupCollectionsEndpoints({
collections,
rootCollection: OUR_ANALYTICS,
});
setupCollectionByIdEndpoint({
collections: [PERSONAL_COLLECTION_BASE, TEST_COLLECTION],
});
setupDatabasesEndpoints(databases);
setupSearchEndpoints(models);
setupCollectionItemsEndpoint({
collection: createMockCollection(ROOT_COLLECTION),
collection: createMockCollection(OUR_ANALYTICS),
collectionItems: [],
});
fetchMock.get("path:/api/bookmark", []);
......@@ -141,6 +155,13 @@ async function setup({
}),
qb: createMockQueryBuilderState({ card: openQuestionCard }),
entities: createMockEntitiesState({ dashboards: dashboardsForEntities }),
settings: createMockSettingsState({
"uploads-settings": {
db_id: isUploadEnabled ? SAMPLE_DATABASE.id : null,
schema_name: null,
table_prefix: null,
},
}),
});
renderWithProviders(
......@@ -443,67 +464,94 @@ describe("nav > containers > MainNavbar", () => {
});
describe("better onboarding section", () => {
it("should render Metabase learn link to admins", async () => {
await setup({ user: createMockUser({ is_superuser: true }) });
const link = screen.getByRole("link", { name: /How to use Metabase/i });
expect(link).toBeInTheDocument();
expect(link).toHaveAttribute("href", "https://www.metabase.com/learn/");
});
describe("add data section", () => {
it("should render for admins", async () => {
await setup({
withAdditionalDatabase: false,
user: createMockUser({ is_superuser: true }),
});
it("should render Metabase learn link to regular users", async () => {
await setup({ user: createMockUser({ is_superuser: false }) });
const link = screen.getByRole("link", { name: /How to use Metabase/i });
expect(link).toBeInTheDocument();
expect(link).toHaveAttribute("href", "https://www.metabase.com/learn/");
});
const dataSection = screen.getByTestId("sidebar-add-data-section");
expect(dataSection).toBeInTheDocument();
it("data section should not render to non-admins", async () => {
await setup({ user: createMockUser({ is_superuser: false }) });
const introCTA = screen.getByText(
"Start by adding your data. Connect to a database or upload a CSV file.",
);
expect(introCTA).toBeInTheDocument();
const introCTA = screen.queryByText(
"Start by adding your data. Connect to a database or upload a CSV file.",
);
const addDataButton = screen.queryByRole("button", { name: /Add data/i });
const addDataButton = screen.getByRole("button", { name: /Add data/i });
expect(addDataButton).toBeInTheDocument();
await userEvent.click(addDataButton);
expect(introCTA).not.toBeInTheDocument();
expect(addDataButton).not.toBeInTheDocument();
});
const menu = screen.getByRole("menu");
const menuItems = screen.getAllByRole("menuitem");
it("intro CTA should not render when there are databases connected", async () => {
await setup({
hasOwnDatabase: true,
user: createMockUser({ is_superuser: true }),
expect(menuItems).toHaveLength(2);
expect(within(menu).getAllByRole("link")).toHaveLength(1);
});
const introCTA = screen.queryByText(
"Start by adding your data. Connect to a database or upload a CSV file.",
);
expect(introCTA).not.toBeInTheDocument();
});
it("intro CTA should not render when there are databases connected", async () => {
await setup({
withAdditionalDatabase: true,
user: createMockUser({ is_superuser: true }),
});
it("data section should render for admins", async () => {
await setup({
hasOwnDatabase: false,
user: createMockUser({ is_superuser: true }),
const introCTA = screen.queryByText(
"Start by adding your data. Connect to a database or upload a CSV file.",
);
expect(introCTA).not.toBeInTheDocument();
});
const introCTA = screen.getByText(
"Start by adding your data. Connect to a database or upload a CSV file.",
);
const addDataButton = screen.getByRole("button", { name: /Add data/i });
it("should render for regular users with 'curate' root collection permissions if uploads are disabled, but they have general data access", async () => {
await setup({
user: createMockUser({ is_superuser: false }),
canCurateRootCollection: true,
hasDataAccess: true,
isUploadEnabled: false,
});
const dataSection = screen.getByTestId("sidebar-add-data-section");
expect(dataSection).toBeInTheDocument();
});
it("should render for regular users with 'curate' root collection permissions if uploads are enabled for a database and they can upload to that database", async () => {
await setup({
user: createMockUser({ is_superuser: false }),
canCurateRootCollection: true,
hasDataAccess: true,
isUploadEnabled: true,
});
const dataSection = screen.getByTestId("sidebar-add-data-section");
expect(dataSection).toBeInTheDocument();
});
it("should not render for regular users if they cannot 'curate' the root collection", async () => {
await setup({
user: createMockUser({ is_superuser: false }),
canCurateRootCollection: false,
hasDataAccess: true,
isUploadEnabled: true,
});
expect(introCTA).toBeInTheDocument();
expect(addDataButton).toBeInTheDocument();
await userEvent.click(addDataButton);
const dataSection = screen.queryByTestId("sidebar-add-data-section");
expect(dataSection).not.toBeInTheDocument();
});
const menu = screen.getByRole("menu");
const menuItems = screen.getAllByRole("menuitem");
it("should not render for regular users if they don't have data access", async () => {
await setup({
user: createMockUser({ is_superuser: false }),
canCurateRootCollection: true,
hasDataAccess: false,
isUploadEnabled: true,
});
expect(menuItems).toHaveLength(2);
expect(within(menu).getAllByRole("link")).toHaveLength(1);
const dataSection = screen.queryByTestId("sidebar-add-data-section");
expect(dataSection).not.toBeInTheDocument();
});
});
describe("'Add a database' menu option", () => {
describe("'Add a database' menu item", () => {
it("should be wrapped in a link", async () => {
await setup({
user: createMockUser({ is_superuser: true }),
......@@ -522,7 +570,7 @@ describe("nav > containers > MainNavbar", () => {
);
});
it("should render", async () => {
it("should render properly for admins", async () => {
await setup({
user: createMockUser({ is_superuser: true }),
});
......@@ -544,10 +592,29 @@ describe("nav > containers > MainNavbar", () => {
within(addDatabaseMenuItem).getByLabelText("database icon"),
).toBeInTheDocument();
});
it("should not render for regular users", async () => {
await setup({
user: createMockUser({ is_superuser: false }),
canCurateRootCollection: true,
hasDataAccess: true,
isUploadEnabled: true,
});
const addDataButton = screen.getByRole("button", { name: /Add data/i });
await userEvent.click(addDataButton);
const menuItems = screen.queryAllByRole("menuitem");
const [menuItem] = menuItems;
expect(menuItems).toHaveLength(1);
expect(
within(menuItem).queryByText("Add a database"),
).not.toBeInTheDocument();
});
});
describe("'Upload a spreadsheet' menu option", () => {
it("should render", async () => {
describe("'Upload a spreadsheet' menu item", () => {
it("should render properly for admins", async () => {
await setup({
user: createMockUser({ is_superuser: true }),
});
......@@ -567,9 +634,33 @@ describe("nav > containers > MainNavbar", () => {
).toBeInTheDocument();
});
it("clicking on it should open a CSV info modal", async () => {
it("should render properly for regular users", async () => {
await setup({
user: createMockUser({ is_superuser: false }),
canCurateRootCollection: true,
hasDataAccess: true,
isUploadEnabled: true,
});
const addDataButton = screen.getByRole("button", { name: /Add data/i });
await userEvent.click(addDataButton);
const [uploadCSVMenuItem] = screen.getAllByRole("menuitem");
expect(
within(uploadCSVMenuItem).getByText("Upload a spreadsheet"),
).toBeInTheDocument();
expect(
within(uploadCSVMenuItem).getByText(".csv, .tsv (50 MB max)"),
).toBeInTheDocument();
expect(
within(uploadCSVMenuItem).getByLabelText("table2 icon"),
).toBeInTheDocument();
});
it("clicking on it should show a CSV info modal for an admin, if uploads are disabled", async () => {
await setup({
user: createMockUser({ is_superuser: true }),
isUploadEnabled: false,
});
const addDataButton = screen.getByRole("button", { name: /Add data/i });
......@@ -580,7 +671,92 @@ describe("nav > containers > MainNavbar", () => {
await userEvent.click(uploadCSVMenuItem);
expect(menu).not.toBeInTheDocument();
expect(screen.getByText("Upload CSVs to Metabase")).toBeInTheDocument();
const modal = screen.getByRole("dialog");
expect(modal).toBeInTheDocument();
[
"Upload CSVs to Metabase",
"Team members will be able to upload CSV files and work with them just like any other data source.",
"You'll be able to pick the default database where the data should be stored when enabling the feature.",
"Go to setup",
].forEach(copy => {
expect(within(modal).getByText(copy)).toBeInTheDocument();
});
expect(within(modal).getByRole("link")).toHaveAttribute(
"href",
"/admin/settings/uploads",
);
});
it("clicking on it should show a CSV info modal for a regular user, if uploads are disabled", async () => {
await setup({
user: createMockUser({ is_superuser: false }),
canCurateRootCollection: true,
hasDataAccess: true,
isUploadEnabled: false,
});
const addDataButton = screen.getByRole("button", { name: /Add data/i });
await userEvent.click(addDataButton);
const menu = screen.getByRole("menu");
const [uploadCSVMenuItem] = screen.getAllByRole("menuitem");
await userEvent.click(uploadCSVMenuItem);
expect(menu).not.toBeInTheDocument();
const modal = screen.getByRole("dialog");
expect(modal).toBeInTheDocument();
[
"Upload CSVs to Metabase",
"You'll need to ask your admin to enable this feature to get started. Then, you'll be able to upload CSV files and work with them just like any other data source.",
"Got it",
].forEach(copy => {
expect(within(modal).getByText(copy)).toBeInTheDocument();
});
expect(within(modal).queryByRole("link")).not.toBeInTheDocument();
});
it("clicking on it should not show a CSV info modal for an admin, if uploads are enabled", async () => {
await setup({
user: createMockUser({ is_superuser: true }),
isUploadEnabled: true,
});
const addDataButton = screen.getByRole("button", { name: /Add data/i });
await userEvent.click(addDataButton);
const menu = screen.getByRole("menu");
const [_, uploadCSVMenuItem] = screen.getAllByRole("menuitem");
await userEvent.click(uploadCSVMenuItem);
expect(menu).not.toBeInTheDocument();
const modal = screen.queryByRole("dialog");
expect(modal).not.toBeInTheDocument();
});
it("clicking on it should not show a CSV info modal for a regular user, if uploads are enabled", async () => {
await setup({
user: createMockUser({ is_superuser: false }),
canCurateRootCollection: true,
hasDataAccess: true,
isUploadEnabled: true,
});
const addDataButton = screen.getByRole("button", { name: /Add data/i });
await userEvent.click(addDataButton);
const menu = screen.getByRole("menu");
const [uploadCSVMenuItem] = screen.getAllByRole("menuitem");
await userEvent.click(uploadCSVMenuItem);
expect(menu).not.toBeInTheDocument();
const modal = screen.queryByRole("dialog");
expect(modal).not.toBeInTheDocument();
});
});
});
......
......@@ -23,7 +23,7 @@ import Collections, {
} from "metabase/entities/collections";
import Databases from "metabase/entities/databases";
import * as Urls from "metabase/lib/urls";
import { getHasDataAccess, getHasOwnDatabase } from "metabase/selectors/data";
import { getHasDataAccess } from "metabase/selectors/data";
import { getUser, getUserIsAdmin } from "metabase/selectors/user";
import type Database from "metabase-lib/v1/metadata/Database";
import type { Bookmark, Collection, User } from "metabase-types/api";
......@@ -42,7 +42,6 @@ function mapStateToProps(state: State, { databases = [] }: DatabaseProps) {
currentUser: getUser(state),
isAdmin: getUserIsAdmin(state),
hasDataAccess: getHasDataAccess(databases),
hasOwnDatabase: getHasOwnDatabase(databases),
bookmarks: getOrderedBookmarks(state),
};
}
......@@ -55,11 +54,11 @@ const mapDispatchToProps = {
interface Props extends MainNavbarProps {
isAdmin: boolean;
currentUser: User;
databases: Database[];
selectedItems: SelectedItem[];
bookmarks: Bookmark[];
rootCollection: Collection;
hasDataAccess: boolean;
hasOwnDatabase: boolean;
allError: boolean;
allFetched: boolean;
logout: () => void;
......@@ -77,7 +76,6 @@ function MainNavbarContainer({
selectedItems,
isOpen,
currentUser,
hasOwnDatabase,
rootCollection,
hasDataAccess,
location,
......@@ -191,7 +189,6 @@ function MainNavbarContainer({
isOpen={isOpen}
currentUser={currentUser}
collections={collectionTree}
hasOwnDatabase={hasOwnDatabase}
selectedItems={selectedItems}
hasDataAccess={hasDataAccess}
reorderBookmarks={reorderBookmarks}
......
......@@ -16,6 +16,7 @@ import { isSmallScreen } from "metabase/lib/dom";
import * as Urls from "metabase/lib/urls";
import { WhatsNewNotification } from "metabase/nav/components/WhatsNewNotification";
import type { IconName, IconProps } from "metabase/ui";
import type Database from "metabase-lib/v1/metadata/Database";
import type { Bookmark, Collection, User } from "metabase-types/api";
import {
......@@ -46,8 +47,8 @@ type Props = {
currentUser: User;
bookmarks: Bookmark[];
hasDataAccess: boolean;
hasOwnDatabase: boolean;
collections: CollectionTreeItem[];
databases: Database[];
selectedItems: SelectedItem[];
handleCloseNavbar: () => void;
handleLogout: () => void;
......@@ -67,7 +68,7 @@ export function MainNavbarView({
currentUser,
bookmarks,
collections,
hasOwnDatabase,
databases,
selectedItems,
hasDataAccess,
reorderBookmarks,
......@@ -183,7 +184,9 @@ export function MainNavbarView({
</div>
<WhatsNewNotification />
<SidebarOnboardingSection
hasOwnDatabase={hasOwnDatabase}
collections={collections}
databases={databases}
hasDataAccess={hasDataAccess}
isAdmin={isAdmin}
/>
</SidebarContentRoot>
......
......@@ -11,7 +11,6 @@ import {
} from "metabase/collections/components/ModelUploadModal";
import type { OnFileUpload } from "metabase/collections/types";
import { UploadInput } from "metabase/components/upload";
import ExternalLink from "metabase/core/components/ExternalLink";
import Link from "metabase/core/components/Link";
import CS from "metabase/css/core/index.css";
import { useToggle } from "metabase/hooks/use-toggle";
......@@ -22,21 +21,22 @@ import {
type UploadFileProps,
uploadFile as uploadFileAction,
} from "metabase/redux/uploads";
import { getLearnUrl, getSetting } from "metabase/selectors/settings";
import { getApplicationName } from "metabase/selectors/whitelabel";
import { getHasOwnDatabase } from "metabase/selectors/data";
import { getSetting } from "metabase/selectors/settings";
import { Box, Button, Icon, Menu, Stack, Text, Title } from "metabase/ui";
import { breakpoints } from "metabase/ui/theme";
import { PaddedSidebarLink } from "../../MainNavbar.styled";
import { trackAddDataViaCSV, trackAddDataViaDatabase } from "./analytics";
import type { OnboaringMenuItemProps, SidebarOnboardingProps } from "./types";
export function SidebarOnboardingSection({
hasOwnDatabase,
collections,
databases,
hasDataAccess,
isAdmin,
}: SidebarOnboardingProps) {
const initialState = !hasOwnDatabase;
const isDatabaseAdded = getHasOwnDatabase(databases);
const showCTASection = !isDatabaseAdded;
const [
isModelUploadModalOpen,
......@@ -46,11 +46,9 @@ export function SidebarOnboardingSection({
const [showInfoModal, setShowInfoModal] = useState(false);
const [uploadedFile, setUploadedFile] = useState<File | null>(null);
const applicationName = useSelector(getApplicationName);
const uploadDbId = useSelector(
state => getSetting(state, "uploads-settings")?.db_id,
);
const isUploadEnabled = !!uploadDbId;
const uploadInputRef = useRef<HTMLInputElement>(null);
......@@ -94,29 +92,45 @@ export function SidebarOnboardingSection({
const isMobileSafe = useMediaQuery(`(min-width: ${breakpoints.sm})`);
const canAddDatabase = isAdmin;
/**
* the user must have:
* - "write" permissions for the root collection AND
* - either:
* a) !uploadsEnabled => data access to any of the databases OR
* b) uploadsEnabled => "upload" permissions for the database for which uploads are enabled
*/
const isUploadEnabled = !!uploadDbId;
const rootCollection = collections.find(
c => c.id === "root" || c.id === null,
);
const canCurateRootCollection = rootCollection?.can_write;
const canUploadToDatabase = databases
?.find(db => db.id === uploadDbId)
?.canUpload();
const canUpload =
canCurateRootCollection &&
(isUploadEnabled ? canUploadToDatabase : hasDataAccess);
const handleSpreadsheetButtonClick = isUploadEnabled
? () => uploadInputRef.current?.click()
: () => setShowInfoModal(true);
return (
<Box
m={0}
bottom={0}
pos="sticky"
bg="bg-white"
className={cx({ [CS.borderTop]: !initialState })}
className={cx({ [CS.borderTop]: showCTASection })}
>
<Box px="md" py="md">
{/*eslint-disable-next-line no-unconditional-metabase-links-render -- This link is only temporary. It will be replaced with an internal link to a page. */}
<ExternalLink href={getLearnUrl()} className={CS.noDecoration}>
{/* TODO: We currently don't have a `selected` state. Will be added in MS2 when we add the onboarding page. */}
<PaddedSidebarLink icon="learn">
{t`How to use ${applicationName}`}
</PaddedSidebarLink>
</ExternalLink>
</Box>
{isAdmin && (
<Box px="xl" pb="md" className={cx({ [CS.borderTop]: initialState })}>
{initialState && (
{canAddDatabase || canUpload ? (
<Box px="xl" py="md" data-testid="sidebar-add-data-section">
{showCTASection && (
<Text
fz="sm"
my="md"
mb="md"
lh="1.333"
>{t`Start by adding your data. Connect to a database or upload a CSV file.`}</Text>
)}
......@@ -129,48 +143,29 @@ export function SidebarOnboardingSection({
>{t`Add data`}</Button>
</Menu.Target>
<Menu.Dropdown>
<Link to="/admin/databases/create">
<SidebarOnboardingMenuItem
icon="database"
title={t`Add a database`}
subtitle={t`PostgreSQL, MySQL, Snowflake, ...`}
onClick={() => trackAddDataViaDatabase()}
/>
</Link>
{!isUploadEnabled ? (
<SidebarOnboardingMenuItem
icon="table2"
title={t`Upload a spreadsheet`}
subtitle={t`${UPLOAD_DATA_FILE_TYPES.join(
", ",
)} (${MAX_UPLOAD_STRING} MB max)`}
onClick={() => setShowInfoModal(true)}
/>
) : (
<SidebarOnboardingMenuItem
icon="table2"
title={t`Upload a spreadsheet`}
subtitle={t`${UPLOAD_DATA_FILE_TYPES.join(
", ",
)} (${MAX_UPLOAD_STRING} MB max)`}
onClick={() => uploadInputRef.current?.click()}
{canAddDatabase && <AddDatabaseButton />}
{canUpload && (
<UploadSpreadsheetButton
onClick={handleSpreadsheetButtonClick}
/>
)}
</Menu.Dropdown>
</Menu>
</Box>
)}
) : null}
{showInfoModal && (
<UploadInfoModal
isAdmin={isAdmin}
onClose={() => setShowInfoModal(false)}
/>
)}
<UploadInput
id="onboarding-upload-input"
ref={uploadInputRef}
onChange={handleFileInput}
/>
{canUpload && (
<UploadInput
id="onboarding-upload-input"
ref={uploadInputRef}
onChange={handleFileInput}
/>
)}
<ModelUploadModal
collectionId="root"
opened={isModelUploadModalOpen}
......@@ -204,3 +199,31 @@ function SidebarOnboardingMenuItem({
</Menu.Item>
);
}
function AddDatabaseButton() {
return (
<Link to="/admin/databases/create">
<SidebarOnboardingMenuItem
icon="database"
title={t`Add a database`}
subtitle={t`PostgreSQL, MySQL, Snowflake, ...`}
onClick={() => trackAddDataViaDatabase()}
/>
</Link>
);
}
function UploadSpreadsheetButton({ onClick }: { onClick: () => void }) {
const subtitle = t`${UPLOAD_DATA_FILE_TYPES.join(
", ",
)} (${MAX_UPLOAD_STRING} MB max)`;
return (
<SidebarOnboardingMenuItem
icon="table2"
title={t`Upload a spreadsheet`}
subtitle={subtitle}
onClick={onClick}
/>
);
}
import type { CollectionTreeItem } from "metabase/entities/collections/utils";
import type { IconName } from "metabase/ui";
import type Database from "metabase-lib/v1/metadata/Database";
export type SidebarOnboardingProps = {
hasOwnDatabase: boolean;
collections: CollectionTreeItem[];
databases: Database[];
hasDataAccess: boolean;
isAdmin: boolean;
};
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment