From ad5da72d758e1bed851cda4189fad97945b6874e Mon Sep 17 00:00:00 2001
From: Ryan Laurie <30528226+iethree@users.noreply.github.com>
Date: Thu, 5 Oct 2023 07:22:44 -0600
Subject: [PATCH] Convert collection permissions to typescript and add unit
tests (#34186)
---
.../src/metabase-types/api/permissions.ts | 18 +-
frontend/src/metabase-types/store/admin.ts | 8 +-
.../src/metabase-types/store/collection.ts | 17 +
frontend/src/metabase-types/store/index.ts | 1 +
.../src/metabase-types/store/mocks/admin.ts | 2 +
.../PermissionsSidebar/PermissionsSidebar.tsx | 2 +-
.../PermissionsSidebarContent.tsx | 10 +-
.../PermissionsTable/PermissionsTable.jsx | 6 +-
...Page.jsx => CollectionPermissionsPage.tsx} | 72 +++--
.../CollectionPermissionsPage.unit.spec.tsx | 305 ++++++++++++++++++
.../src/metabase/admin/permissions/routes.jsx | 2 +-
...rmissions.js => collection-permissions.ts} | 137 ++++++--
.../metabase/entities/collections/utils.ts | 2 +-
.../__support__/server-mocks/permissions.ts | 19 +-
14 files changed, 531 insertions(+), 70 deletions(-)
create mode 100644 frontend/src/metabase-types/store/collection.ts
rename frontend/src/metabase/admin/permissions/pages/CollectionPermissionsPage/{CollectionPermissionsPage.jsx => CollectionPermissionsPage.tsx} (66%)
create mode 100644 frontend/src/metabase/admin/permissions/pages/CollectionPermissionsPage/CollectionPermissionsPage.unit.spec.tsx
rename frontend/src/metabase/admin/permissions/selectors/{collection-permissions.js => collection-permissions.ts} (62%)
diff --git a/frontend/src/metabase-types/api/permissions.ts b/frontend/src/metabase-types/api/permissions.ts
index 49bfb04f19f..c9a7d6ca0d2 100644
--- a/frontend/src/metabase-types/api/permissions.ts
+++ b/frontend/src/metabase-types/api/permissions.ts
@@ -1,4 +1,9 @@
-import type { DatabaseId, TableId, SchemaName } from "metabase-types/api";
+import type {
+ DatabaseId,
+ TableId,
+ SchemaName,
+ CollectionId,
+} from "metabase-types/api";
import type { GroupId } from "./group";
import type { UserAttribute } from "./user";
@@ -78,6 +83,17 @@ export type FieldsPermissions =
query: "segmented";
};
+export type CollectionPermissionsGraph = {
+ groups: CollectionPermissions;
+ revision: number;
+};
+
+export type CollectionPermissions = {
+ [key: GroupId]: Partial<Record<CollectionId, CollectionPermission>>;
+};
+
+export type CollectionPermission = "write" | "read" | "none";
+
// FIXME: is there a more suitable type for this?
export type DimensionRef = ["dimension", any[]];
diff --git a/frontend/src/metabase-types/store/admin.ts b/frontend/src/metabase-types/store/admin.ts
index ffc7c656268..aabcf046c0d 100644
--- a/frontend/src/metabase-types/store/admin.ts
+++ b/frontend/src/metabase-types/store/admin.ts
@@ -1,4 +1,8 @@
-import type { GroupsPermissions, SettingDefinition } from "metabase-types/api";
+import type {
+ CollectionPermissions,
+ GroupsPermissions,
+ SettingDefinition,
+} from "metabase-types/api";
export type AdminPathKey =
| "data-model"
@@ -21,6 +25,8 @@ export interface AdminState {
permissions: {
dataPermissions: GroupsPermissions;
originalDataPermissions: GroupsPermissions;
+ collectionPermissions: CollectionPermissions;
+ originalCollectionPermissions: CollectionPermissions;
saveError?: string;
isHelpReferenceOpen: boolean;
};
diff --git a/frontend/src/metabase-types/store/collection.ts b/frontend/src/metabase-types/store/collection.ts
new file mode 100644
index 00000000000..42445f6b745
--- /dev/null
+++ b/frontend/src/metabase-types/store/collection.ts
@@ -0,0 +1,17 @@
+import type { Collection, CollectionId } from "metabase-types/api";
+import type { IconName } from "metabase/core/components/Icon";
+
+// see entities/collections/getExpandedCollectionsById.js
+export type ExpandedCollection = Collection & {
+ path: string;
+ parent: ExpandedCollection;
+ children: ExpandedCollection[];
+ is_personal: boolean;
+};
+
+export type CollectionTreeItem = {
+ id: CollectionId;
+ name: string;
+ icon: IconName | any;
+ children?: CollectionTreeItem[];
+};
diff --git a/frontend/src/metabase-types/store/index.ts b/frontend/src/metabase-types/store/index.ts
index c4268a931e0..4bce48841c9 100644
--- a/frontend/src/metabase-types/store/index.ts
+++ b/frontend/src/metabase-types/store/index.ts
@@ -1,6 +1,7 @@
export * from "./admin";
export * from "./app";
export * from "./auth";
+export * from "./collection";
export * from "./dashboard";
export * from "./embed";
export * from "./entities";
diff --git a/frontend/src/metabase-types/store/mocks/admin.ts b/frontend/src/metabase-types/store/mocks/admin.ts
index c1f23a36d6f..adc5731f34e 100644
--- a/frontend/src/metabase-types/store/mocks/admin.ts
+++ b/frontend/src/metabase-types/store/mocks/admin.ts
@@ -23,6 +23,8 @@ export const createMockPermissionsState = (
return {
dataPermissions: {},
originalDataPermissions: {},
+ collectionPermissions: {},
+ originalCollectionPermissions: {},
isHelpReferenceOpen: false,
...opts,
};
diff --git a/frontend/src/metabase/admin/permissions/components/PermissionsSidebar/PermissionsSidebar.tsx b/frontend/src/metabase/admin/permissions/components/PermissionsSidebar/PermissionsSidebar.tsx
index 82ee6484200..5c432863806 100644
--- a/frontend/src/metabase/admin/permissions/components/PermissionsSidebar/PermissionsSidebar.tsx
+++ b/frontend/src/metabase/admin/permissions/components/PermissionsSidebar/PermissionsSidebar.tsx
@@ -5,7 +5,7 @@ import { SidebarRoot } from "./PermissionsSidebar.styled";
interface PermissionsSidebarProps extends PermissionsSidebarContentProps {
isLoading?: boolean;
- error: string;
+ error?: string;
}
export const PermissionsSidebar = ({
diff --git a/frontend/src/metabase/admin/permissions/components/PermissionsSidebar/PermissionsSidebarContent.tsx b/frontend/src/metabase/admin/permissions/components/PermissionsSidebar/PermissionsSidebarContent.tsx
index bc718a5bcfd..be17fc1885d 100644
--- a/frontend/src/metabase/admin/permissions/components/PermissionsSidebar/PermissionsSidebarContent.tsx
+++ b/frontend/src/metabase/admin/permissions/components/PermissionsSidebar/PermissionsSidebarContent.tsx
@@ -19,11 +19,11 @@ export interface PermissionsSidebarContentProps {
description?: string;
filterPlaceholder: string;
onSelect: (item: ITreeNodeItem) => void;
- onBack: () => void;
- selectedId: ITreeNodeItem["id"];
+ onBack?: () => void;
+ selectedId?: ITreeNodeItem["id"];
entityGroups: ITreeNodeItem[][];
- onEntityChange: (entity: string) => void;
- entityViewFocus: "database" | "group";
+ onEntityChange?: (entity: string) => void;
+ entityViewFocus?: "database" | "group";
}
export const PermissionsSidebarContent = memo<PermissionsSidebarContentProps>(
@@ -50,7 +50,7 @@ export const PermissionsSidebarContent = memo<PermissionsSidebarContentProps>(
<SidebarContentTitle>{title}</SidebarContentTitle>
)}
{description && <Text color="text-dark">{description}</Text>}
- {entityViewFocus && (
+ {entityViewFocus && onEntityChange && (
<EntityViewSwitch
value={entityViewFocus}
onChange={onEntityChange}
diff --git a/frontend/src/metabase/admin/permissions/components/PermissionsTable/PermissionsTable.jsx b/frontend/src/metabase/admin/permissions/components/PermissionsTable/PermissionsTable.jsx
index 31b4ff73fac..1a10db10ff0 100644
--- a/frontend/src/metabase/admin/permissions/components/PermissionsTable/PermissionsTable.jsx
+++ b/frontend/src/metabase/admin/permissions/components/PermissionsTable/PermissionsTable.jsx
@@ -120,9 +120,11 @@ export function PermissionsTable({
)}
</PermissionsTableCell>
- {entity.permissions.map(permission => {
+ {entity.permissions.map((permission, index) => {
return (
- <PermissionsTableCell key={permission.type}>
+ <PermissionsTableCell
+ key={permission.type ?? String(index)}
+ >
<PermissionsSelect
{...permission}
onChange={(value, toggleState) =>
diff --git a/frontend/src/metabase/admin/permissions/pages/CollectionPermissionsPage/CollectionPermissionsPage.jsx b/frontend/src/metabase/admin/permissions/pages/CollectionPermissionsPage/CollectionPermissionsPage.tsx
similarity index 66%
rename from frontend/src/metabase/admin/permissions/pages/CollectionPermissionsPage/CollectionPermissionsPage.jsx
rename to frontend/src/metabase/admin/permissions/pages/CollectionPermissionsPage/CollectionPermissionsPage.tsx
index a30c88d3bc8..9cf65a18080 100644
--- a/frontend/src/metabase/admin/permissions/pages/CollectionPermissionsPage/CollectionPermissionsPage.jsx
+++ b/frontend/src/metabase/admin/permissions/pages/CollectionPermissionsPage/CollectionPermissionsPage.tsx
@@ -1,10 +1,13 @@
import { useEffect, useCallback } from "react";
-import PropTypes from "prop-types";
import { push } from "react-router-redux";
import { connect } from "react-redux";
+import type { Route } from "react-router";
import { t } from "ttag";
import _ from "underscore";
+import type { Collection, CollectionId, GroupId } from "metabase-types/api";
+import type { State } from "metabase-types/store";
+
import Groups from "metabase/entities/groups";
import Collections from "metabase/entities/collections";
import { CollectionPermissionsHelp } from "metabase/admin/permissions/components/CollectionPermissionsHelp";
@@ -12,8 +15,8 @@ import { CollectionPermissionsHelp } from "metabase/admin/permissions/components
import {
PermissionsEditor,
PermissionsEditorEmptyState,
- permissionEditorPropTypes,
} from "../../components/PermissionsEditor";
+
import PermissionsPageLayout from "../../components/PermissionsPageLayout";
import {
initializeCollectionPermissions,
@@ -21,6 +24,12 @@ import {
saveCollectionPermissions,
loadCollectionPermissions,
} from "../../permissions";
+
+import type {
+ CollectionIdProps,
+ CollectionPermissionEditorType,
+ CollectionSidebarType,
+} from "../../selectors/collection-permissions";
import {
getCollectionsSidebar,
getCollectionsPermissionEditor,
@@ -28,43 +37,54 @@ import {
getIsDirty,
collectionsQuery,
} from "../../selectors/collection-permissions";
+
import { PermissionsSidebar } from "../../components/PermissionsSidebar";
const mapDispatchToProps = {
initialize: initializeCollectionPermissions,
loadPermissions: loadCollectionPermissions,
- navigateToItem: ({ id }) => push(`/admin/permissions/collections/${id}`),
+ navigateToItem: ({ id }: { id: CollectionId }) =>
+ push(`/admin/permissions/collections/${id}`),
updateCollectionPermission,
savePermissions: saveCollectionPermissions,
};
-const mapStateToProps = (state, props) => {
+const mapStateToProps = (state: State, props: CollectionIdProps) => {
return {
sidebar: getCollectionsSidebar(state, props),
permissionEditor: getCollectionsPermissionEditor(state, props),
- isDirty: getIsDirty(state, props),
+ isDirty: getIsDirty(state),
collection: getCollectionEntity(state, props),
};
};
-const propTypes = {
- params: PropTypes.shape({
- collectionId: PropTypes.string,
- }),
- children: PropTypes.node.isRequired,
- sidebar: PropTypes.object,
- permissionEditor: PropTypes.shape(permissionEditorPropTypes),
- collection: PropTypes.object,
- navigateToItem: PropTypes.func.isRequired,
- updateCollectionPermission: PropTypes.func.isRequired,
- isDirty: PropTypes.bool,
- savePermissions: PropTypes.func.isRequired,
- loadPermissions: PropTypes.func.isRequired,
- initialize: PropTypes.func.isRequired,
- route: PropTypes.object,
+type UpdateCollectionPermissionParams = {
+ groupId: GroupId;
+ collection: Collection;
+ value: unknown;
+ shouldPropagate: boolean;
+};
+
+type CollectionPermissionsPageProps = {
+ params: CollectionIdProps["params"];
+ sidebar: CollectionSidebarType;
+ permissionEditor: CollectionPermissionEditorType;
+ collection: Collection;
+ navigateToItem: (item: any) => void;
+ updateCollectionPermission: ({
+ groupId,
+ collection,
+ value,
+ shouldPropagate,
+ }: UpdateCollectionPermissionParams) => void;
+ isDirty: boolean;
+ savePermissions: () => void;
+ loadPermissions: () => void;
+ initialize: () => void;
+ route: Route;
};
-function CollectionsPermissionsPage({
+function CollectionsPermissionsPageView({
sidebar,
permissionEditor,
collection,
@@ -75,7 +95,7 @@ function CollectionsPermissionsPage({
navigateToItem,
initialize,
route,
-}) {
+}: CollectionPermissionsPageProps) {
useEffect(() => {
initialize();
}, [initialize]);
@@ -112,6 +132,8 @@ function CollectionsPermissionsPage({
{permissionEditor && (
<PermissionsEditor
+ isLoading={undefined}
+ error={undefined}
{...permissionEditor}
onChange={handlePermissionChange}
/>
@@ -120,12 +142,10 @@ function CollectionsPermissionsPage({
);
}
-CollectionsPermissionsPage.propTypes = propTypes;
-
-export default _.compose(
+export const CollectionPermissionsPage = _.compose(
Collections.loadList({
entityQuery: collectionsQuery,
}),
Groups.loadList(),
connect(mapStateToProps, mapDispatchToProps),
-)(CollectionsPermissionsPage);
+)(CollectionsPermissionsPageView);
diff --git a/frontend/src/metabase/admin/permissions/pages/CollectionPermissionsPage/CollectionPermissionsPage.unit.spec.tsx b/frontend/src/metabase/admin/permissions/pages/CollectionPermissionsPage/CollectionPermissionsPage.unit.spec.tsx
new file mode 100644
index 00000000000..b237e413eeb
--- /dev/null
+++ b/frontend/src/metabase/admin/permissions/pages/CollectionPermissionsPage/CollectionPermissionsPage.unit.spec.tsx
@@ -0,0 +1,305 @@
+import { Route } from "react-router";
+import { within } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import fetchMock from "fetch-mock";
+
+import { renderWithProviders, screen } from "__support__/ui";
+import { createMockState } from "metabase-types/store/mocks";
+
+import { createMockCollection } from "metabase-types/api/mocks";
+
+import {
+ setupCollectionPermissionsGraphEndpoint,
+ setupCollectionsEndpoints,
+ setupGroupsEndpoint,
+} from "__support__/server-mocks";
+
+import type { CollectionPermissionsGraph } from "metabase-types/api";
+import { CollectionPermissionsPage } from "./CollectionPermissionsPage";
+
+const personalCollection = createMockCollection({
+ id: "personal",
+ name: "Personal",
+ personal_owner_id: 1,
+});
+
+const nestedCollectionOne = createMockCollection({
+ id: 3,
+ name: "Nested One",
+ location: "/1/",
+ children: [],
+});
+const nestedCollectionTwo = createMockCollection({
+ id: 4,
+ name: "Nested Two",
+ location: "/2/",
+ children: [],
+});
+
+const collectionOne = createMockCollection({
+ id: 1,
+ name: "Collection One",
+ children: [nestedCollectionOne],
+});
+const collectionTwo = createMockCollection({
+ id: 2,
+ name: "Collection Two",
+ children: [nestedCollectionTwo],
+});
+
+const rootCollection = createMockCollection({
+ id: "root",
+ name: "Our analytics",
+ children: [collectionOne, collectionTwo],
+});
+
+const permissionGroups = [
+ { id: 1, name: "All Users", member_count: 40 },
+ { id: 2, name: "Administrators", member_count: 2 },
+ { id: 3, name: "Other Users", member_count: 33 },
+];
+
+const permissionsGraph: CollectionPermissionsGraph = {
+ revision: 23,
+ groups: {
+ 1: {
+ // all users
+ 1: "write", // one
+ 2: "write", // two
+ 3: "read", // nested one
+ 4: "none", // nested two
+ root: "read",
+ },
+ 2: {
+ // Administrators
+ 1: "write", // one
+ 2: "write", // two
+ 3: "write", // nested one
+ 4: "write", // nested two
+ root: "write",
+ },
+ 3: {
+ // Other users
+ 1: "read", // one
+ 2: "read", // two
+ 3: "none", // nested one
+ 4: "none", // nested two
+ root: "read",
+ },
+ },
+};
+
+function setup() {
+ setupCollectionsEndpoints({
+ collections: [collectionOne, collectionTwo, personalCollection],
+ rootCollection: rootCollection,
+ });
+
+ setupCollectionPermissionsGraphEndpoint(permissionsGraph);
+
+ setupGroupsEndpoint(permissionGroups);
+
+ const initialState = createMockState();
+
+ renderWithProviders(
+ <>
+ <Route
+ path="/admin/permissions/collections/root"
+ component={CollectionPermissionsPage}
+ />
+ <Route
+ path="/admin/permissions/collections/:collectionId"
+ component={CollectionPermissionsPage}
+ />
+ </>,
+ {
+ storeInitialState: initialState,
+ withRouter: true,
+ initialRoute: "/admin/permissions/collections/root",
+ },
+ );
+}
+
+describe("Admin > CollectionPermissionsPage", () => {
+ describe("CollectionPermissionsPage", () => {
+ it("should show a collections tree in the sidebar", async () => {
+ await setup();
+
+ expect(
+ await screen.findByText("Select a collection to see its permissions"),
+ ).toBeVisible();
+ expect(await screen.findByText("Our analytics")).toBeVisible();
+ expect(await screen.findByText("Collection One")).toBeVisible();
+ expect(await screen.findByText("Collection Two")).toBeVisible();
+ });
+
+ it("should allow expansion of nested collections", async () => {
+ await setup();
+
+ const collection1 = await screen.findByText("Collection One");
+ expect(screen.queryByText("Nested One")).not.toBeInTheDocument();
+ userEvent.click(collection1);
+ expect(await screen.findByText("Nested One")).toBeInTheDocument();
+ });
+
+ it("should not show personal collection", async () => {
+ await setup();
+
+ expect(await screen.findByText("Collection One")).toBeInTheDocument();
+ expect(screen.queryByText("Personal")).not.toBeInTheDocument();
+ });
+
+ it("should show a permissions table for the selected collection", async () => {
+ await setup();
+
+ expect(
+ await screen.findByText("Select a collection to see its permissions"),
+ ).toBeVisible();
+
+ userEvent.click(await screen.findByText("Collection One"));
+ userEvent.click(await screen.findByText("Nested One"));
+
+ expect(
+ await screen.findByText("Permissions for Nested One"),
+ ).toBeVisible();
+ expect(await screen.findByText("Administrators")).toBeVisible();
+ expect(await screen.findByText("All Users")).toBeVisible();
+ expect(await screen.findByText("Other Users")).toBeVisible();
+
+ // 1 groups has write, 1 has read, 1 has none
+ expect(await screen.findByText("Curate")).toBeInTheDocument();
+ expect(await screen.findByText("View")).toBeInTheDocument();
+ expect(await screen.findByText("No access")).toBeInTheDocument();
+ });
+
+ it("can change group permissions", async () => {
+ await setup();
+
+ expect(
+ await screen.findByText("Select a collection to see its permissions"),
+ ).toBeVisible();
+
+ userEvent.click(await screen.findByText("Collection One"));
+ userEvent.click(await screen.findByText("Nested One"));
+
+ // change Other users from no access to view
+ userEvent.click(await screen.findByText("No access"));
+ const listbox = await screen.findByRole("listbox");
+ userEvent.click(within(listbox).getByText("View"));
+
+ expect(
+ await screen.findByText("You've made changes to permissions."),
+ ).toBeInTheDocument();
+
+ userEvent.click(await screen.findByText("Save changes"));
+
+ // are you sure you want to save?
+ userEvent.click(await screen.findByText("Yes"));
+
+ expect(
+ await screen.findByText("You've made changes to permissions."),
+ ).not.toBeInTheDocument();
+
+ expect(await screen.findByText("Curate")).toBeInTheDocument();
+ expect(await screen.findAllByText("View")).toHaveLength(2);
+ expect(screen.queryByText("No access")).not.toBeInTheDocument();
+
+ const lastRequest = await fetchMock
+ .lastCall("path:/api/collection/graph", {
+ method: "PUT",
+ })
+ ?.request?.json();
+
+ expect(lastRequest).toEqual({
+ ...permissionsGraph,
+ groups: {
+ ...permissionsGraph.groups,
+ 3: {
+ ...permissionsGraph.groups[3],
+ 3: "read",
+ },
+ },
+ });
+ });
+
+ it("can propagate permissions changes to sub-collection", async () => {
+ await setup();
+
+ expect(
+ await screen.findByText("Select a collection to see its permissions"),
+ ).toBeVisible();
+
+ userEvent.click(await screen.findByText("Collection One"));
+
+ // change other users from read to curate on collection one
+ // should also change permissions on nested one from no access to curate
+ const otherUsersRow = await screen
+ .findAllByRole("row")
+ .then(rows => rows[3]);
+
+ expect(within(otherUsersRow).getByText("Other Users")).toBeVisible();
+ userEvent.click(within(otherUsersRow).getByText("View"));
+
+ const popover = await screen.findByTestId("popover");
+ userEvent.click(within(popover).getByRole("switch")); // propagate switch
+ userEvent.click(within(popover).getByText("Curate"));
+
+ expect(
+ await screen.findByText("You've made changes to permissions."),
+ ).toBeInTheDocument();
+
+ userEvent.click(await screen.findByText("Save changes"));
+
+ // are you sure you want to save?
+ userEvent.click(await screen.findByText("Yes"));
+
+ expect(
+ await screen.findByText("You've made changes to permissions."),
+ ).not.toBeInTheDocument();
+
+ expect(await screen.findAllByText("Curate")).toHaveLength(3);
+ expect(screen.queryByText("No access")).not.toBeInTheDocument();
+
+ const lastRequest = await fetchMock
+ .lastCall("path:/api/collection/graph", {
+ method: "PUT",
+ })
+ ?.request?.json();
+
+ expect(lastRequest).toEqual({
+ ...permissionsGraph,
+ groups: {
+ ...permissionsGraph.groups,
+ 3: {
+ ...permissionsGraph.groups[3],
+ 1: "write",
+ 3: "write",
+ },
+ },
+ });
+ });
+
+ it("should show toggle to change sub-collection permissions if the collection has sub-collections", async () => {
+ await setup();
+
+ userEvent.click(await screen.findByText("Collection One"));
+ userEvent.click(await screen.findByText("View"));
+
+ expect(
+ await screen.findByText("Also change sub-collections"),
+ ).toBeInTheDocument();
+ });
+
+ it("should not show toggle to change sub-collection permissions if the collection does not have sub-collections", async () => {
+ await setup();
+
+ userEvent.click(await screen.findByText("Collection One"));
+ userEvent.click(await screen.findByText("Nested One"));
+ userEvent.click(await screen.findByText("View"));
+
+ expect(
+ screen.queryByText("Also change sub-collections"),
+ ).not.toBeInTheDocument();
+ });
+ });
+});
diff --git a/frontend/src/metabase/admin/permissions/routes.jsx b/frontend/src/metabase/admin/permissions/routes.jsx
index e369a209ae0..086ed8bfcbb 100644
--- a/frontend/src/metabase/admin/permissions/routes.jsx
+++ b/frontend/src/metabase/admin/permissions/routes.jsx
@@ -9,7 +9,7 @@ import {
PLUGIN_ADMIN_PERMISSIONS_DATABASE_ROUTES,
PLUGIN_ADMIN_PERMISSIONS_DATABASE_GROUP_ROUTES,
} from "metabase/plugins";
-import CollectionPermissionsPage from "./pages/CollectionPermissionsPage/CollectionPermissionsPage";
+import { CollectionPermissionsPage } from "./pages/CollectionPermissionsPage/CollectionPermissionsPage";
import DatabasesPermissionsPage from "./pages/DatabasePermissionsPage/DatabasesPermissionsPage";
import GroupsPermissionsPage from "./pages/GroupDataPermissionsPage/GroupsPermissionsPage";
import DataPermissionsPage from "./pages/DataPermissionsPage";
diff --git a/frontend/src/metabase/admin/permissions/selectors/collection-permissions.js b/frontend/src/metabase/admin/permissions/selectors/collection-permissions.ts
similarity index 62%
rename from frontend/src/metabase/admin/permissions/selectors/collection-permissions.js
rename to frontend/src/metabase/admin/permissions/selectors/collection-permissions.ts
index c56b2bdb1e1..de0a159b929 100644
--- a/frontend/src/metabase/admin/permissions/selectors/collection-permissions.js
+++ b/frontend/src/metabase/admin/permissions/selectors/collection-permissions.ts
@@ -16,6 +16,17 @@ import {
isDefaultGroup,
} from "metabase/lib/groups";
+import type {
+ State,
+ ExpandedCollection,
+ CollectionTreeItem,
+} from "metabase-types/store";
+import type {
+ Collection,
+ Group as GroupType,
+ CollectionPermissions,
+ CollectionId,
+} from "metabase-types/api";
import { COLLECTION_OPTIONS } from "../constants/collections-permissions";
import { UNABLE_TO_CHANGE_ADMIN_PERMISSIONS } from "../constants/messages";
import { getPermissionWarningModal } from "./confirmations";
@@ -27,20 +38,30 @@ export const collectionsQuery = {
};
export const getIsDirty = createSelector(
- state => state.admin.permissions.collectionPermissions,
- state => state.admin.permissions.originalCollectionPermissions,
- (permissions, originalPermissions) =>
- JSON.stringify(permissions) !== JSON.stringify(originalPermissions),
+ (state: State) => state.admin.permissions.collectionPermissions,
+ (state: State) => state.admin.permissions.originalCollectionPermissions,
+ (
+ permissions: CollectionPermissions,
+ originalPermissions: CollectionPermissions,
+ ) => JSON.stringify(permissions) !== JSON.stringify(originalPermissions),
);
-export const getCurrentCollectionId = (_state, props) => {
+export type CollectionIdProps = {
+ params: { collectionId: CollectionId };
+ namespace?: string;
+};
+
+export const getCurrentCollectionId = (
+ _state: State,
+ props: CollectionIdProps,
+) => {
if (props.params.collectionId == null) {
- return null;
+ return undefined;
}
return props.params.collectionId === ROOT_COLLECTION.id
? ROOT_COLLECTION.id
- : parseInt(props.params.collectionId);
+ : parseInt(String(props.params.collectionId));
};
const getRootCollectionTreeItem = () => {
@@ -52,7 +73,7 @@ const getRootCollectionTreeItem = () => {
};
};
-const getCollections = state =>
+const getCollections = (state: State) =>
(
Collections.selectors.getList(state, {
entityQuery: collectionsQuery,
@@ -63,24 +84,35 @@ const getCollectionsTree = createSelector([getCollections], collections => {
return [getRootCollectionTreeItem(), ...buildCollectionTree(collections)];
});
-export function buildCollectionTree(collections) {
+export function buildCollectionTree(
+ collections: Collection[] | null,
+): CollectionTreeItem[] {
if (collections == null) {
return [];
}
- return collections.map(collection => {
+ return collections.map((collection: Collection) => {
return {
id: collection.id,
name: collection.name,
icon: getCollectionIcon(collection),
- children: buildCollectionTree(collection.children),
+ children: collection?.children
+ ? buildCollectionTree(collection.children)
+ : [],
};
});
}
+export type CollectionSidebarType = {
+ selectedId?: CollectionId;
+ title: string;
+ entityGroups: [CollectionTreeItem[]];
+ filterPlaceholder: string;
+};
+
export const getCollectionsSidebar = createSelector(
getCollectionsTree,
getCurrentCollectionId,
- (collectionsTree, collectionId) => {
+ (collectionsTree, collectionId): CollectionSidebarType => {
return {
selectedId: collectionId,
title: t`Collections`,
@@ -90,10 +122,13 @@ export const getCollectionsSidebar = createSelector(
},
);
-const getCollectionsPermissions = state =>
+const getCollectionsPermissions = (state: State) =>
state.admin.permissions.collectionPermissions;
-const findCollection = (collections, collectionId) => {
+const findCollection = (
+ collections: Collection[],
+ collectionId: CollectionId,
+): Collection | null => {
if (collections.length === 0) {
return null;
}
@@ -107,7 +142,7 @@ const findCollection = (collections, collectionId) => {
}
return findCollection(
- collections.map(collection => collection.children).flat(),
+ collections.map(collection => collection.children ?? []).flat(),
collectionId,
);
};
@@ -130,35 +165,64 @@ const getCollection = createSelector(
},
);
-const getFolder = (state, props) => {
+const getFolder = (state: State, props: CollectionIdProps) => {
const folderId = getCurrentCollectionId(state, props);
const folders = SnippetCollections.selectors.getList(state);
- return folders.find(folder => folder.id === folderId);
+ return folders.find((folder: Collection) => folder.id === folderId);
};
-export const getCollectionEntity = (state, props) => {
+export const getCollectionEntity = (state: State, props: CollectionIdProps) => {
return props.namespace === "snippets"
? getFolder(state, props)
: getCollection(state, props);
};
-const getCollectionPermission = (permissions, groupId, collectionId) =>
- getIn(permissions, [groupId, collectionId]);
+const getCollectionPermission = (
+ permissions: CollectionPermissions,
+ groupId: number,
+ collectionId: CollectionId,
+) => getIn(permissions, [groupId, collectionId]);
-const getNamespace = (_state, props) => props.namespace;
+const getNamespace = (_state: State, props: CollectionIdProps) =>
+ props.namespace;
-const getToggleLabel = namespace =>
+const getToggleLabel = (namespace?: string) =>
namespace === "snippets"
? t`Also change sub-folders`
: t`Also change sub-collections`;
+export type CollectionPermissionEditorType = null | {
+ title: string;
+ filterPlaceholder: string;
+ columns: [{ name: string }, { name: string }];
+ entities: {
+ id: number;
+ name: string;
+ permissions: {
+ toggleLabel: string;
+ hasChildren: boolean;
+ isDisabled: boolean;
+ disabledTooltip: string | null;
+ value: string;
+ warning: string | null;
+ confirmations: (newValue: string) => string[];
+ options: string[];
+ }[];
+ }[];
+};
+
export const getCollectionsPermissionEditor = createSelector(
getCollectionsPermissions,
getCollectionEntity,
Group.selectors.getList,
getNamespace,
- (permissions, collection, groups, namespace) => {
+ (
+ permissions,
+ collection,
+ groups,
+ namespace,
+ ): CollectionPermissionEditorType => {
if (!permissions || collection == null) {
return null;
}
@@ -167,7 +231,7 @@ export const getCollectionsPermissionEditor = createSelector(
const toggleLabel = hasChildren ? getToggleLabel(namespace) : null;
const defaultGroup = _.find(groups, isDefaultGroup);
- const entities = groups.map(group => {
+ const entities = groups.map((group: GroupType) => {
const isAdmin = isAdminGroup(group);
const defaultGroupPermission = getCollectionPermission(
@@ -176,7 +240,7 @@ export const getCollectionsPermissionEditor = createSelector(
collection.id,
);
- const confirmations = newValue => [
+ const confirmations = (newValue: string) => [
getPermissionWarningModal(
newValue,
defaultGroupPermission,
@@ -223,22 +287,33 @@ export const getCollectionsPermissionEditor = createSelector(
},
);
-const permissionsCollectionFilter = collection => !collection.is_personal;
+const permissionsCollectionFilter = (collection: ExpandedCollection) =>
+ !collection.is_personal;
-function getDecendentCollections(collection) {
+function getDescendentCollections(
+ collection: ExpandedCollection,
+): ExpandedCollection[] {
const subCollections =
collection.children?.filter(permissionsCollectionFilter) || [];
- return subCollections.concat(...subCollections.map(getDecendentCollections));
+ return subCollections.concat(...subCollections.map(getDescendentCollections));
}
-function getPermissionsSet(collections, permissions, groupId) {
+function getPermissionsSet(
+ collections: Collection[],
+ permissions: CollectionPermissions,
+ groupId: number,
+) {
const perms = collections.map(collection =>
getCollectionPermission(permissions, groupId, collection.id),
);
return new Set(perms);
}
-function getCollectionWarning(groupId, collection, permissions) {
+function getCollectionWarning(
+ groupId: number,
+ collection: ExpandedCollection,
+ permissions: CollectionPermissions,
+) {
if (!collection) {
return;
}
@@ -247,7 +322,7 @@ function getCollectionWarning(groupId, collection, permissions) {
groupId,
collection.id,
);
- const descendentCollections = getDecendentCollections(collection);
+ const descendentCollections = getDescendentCollections(collection);
const descendentPerms = getPermissionsSet(
descendentCollections,
permissions,
diff --git a/frontend/src/metabase/entities/collections/utils.ts b/frontend/src/metabase/entities/collections/utils.ts
index 95f67095699..f3ad552124a 100644
--- a/frontend/src/metabase/entities/collections/utils.ts
+++ b/frontend/src/metabase/entities/collections/utils.ts
@@ -20,7 +20,7 @@ export function normalizedCollection(collection: Collection) {
}
export function getCollectionIcon(
- collection: Collection,
+ collection: Partial<Collection>,
{ tooltip = "default" } = {},
): {
name: IconName;
diff --git a/frontend/test/__support__/server-mocks/permissions.ts b/frontend/test/__support__/server-mocks/permissions.ts
index 77ce6be4539..e46f6cba4ae 100644
--- a/frontend/test/__support__/server-mocks/permissions.ts
+++ b/frontend/test/__support__/server-mocks/permissions.ts
@@ -1,8 +1,25 @@
import fetchMock from "fetch-mock";
-import type { PermissionsGraph } from "metabase-types/api";
+import type {
+ CollectionPermissionsGraph,
+ PermissionsGraph,
+} from "metabase-types/api";
export const setupPermissionsGraphEndpoint = (
permissionsGraph: PermissionsGraph,
) => {
fetchMock.get("path:/api/permissions/graph", permissionsGraph);
};
+
+export const setupCollectionPermissionsGraphEndpoint = (
+ permissionsGraph: CollectionPermissionsGraph,
+) => {
+ fetchMock.get("path:/api/collection/graph", permissionsGraph);
+ fetchMock.put(
+ "path:/api/collection/graph",
+ (url: string, opts: any, req: { body: any }) => {
+ const body = JSON.parse(req.body);
+ body.revision += 1;
+ return body;
+ },
+ );
+};
--
GitLab