You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/07/16 22:21:33 UTC
(superset) branch master updated: fix(frontend): fix edit/delete permissions for charts, dashboards, and datasets (#18870) (#32981) (#32995)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 30b93ab7432 fix(frontend): fix edit/delete permissions for charts, dashboards, and datasets (#18870) (#32981) (#32995)
30b93ab7432 is described below
commit 30b93ab7432613cc76b5bd3528393d3147e9eff0
Author: fnardin-maystreet <15...@users.noreply.github.com>
AuthorDate: Fri Jul 17 00:21:26 2026 +0200
fix(frontend): fix edit/delete permissions for charts, dashboards, and datasets (#18870) (#32981) (#32995)
Co-authored-by: Filippo Nardin <fi...@gmail.com>
Co-authored-by: Evan Rusackas <ev...@preset.io>
---
.../spec/helpers/mockBootstrapData.ts | 47 +++++++
.../src/dashboard/components/IconButton.test.tsx | 75 +++++++++++
.../src/dashboard/components/IconButton.tsx | 78 +++++++++---
.../src/dashboard/util/permissionUtils.test.ts | 27 ++++
.../src/dashboard/util/permissionUtils.ts | 19 ++-
.../src/features/charts/ChartCard.tsx | 98 ++++++++++-----
.../src/features/dashboards/DashboardCard.tsx | 82 ++++++++----
.../src/features/home/ChartTable.test.tsx | 6 +
superset-frontend/src/features/home/ChartTable.tsx | 6 +-
.../src/features/home/DashboardTable.test.tsx | 7 ++
.../src/features/home/DashboardTable.tsx | 2 +-
.../pages/ChartList/ChartList.cardview.test.tsx | 5 +
.../pages/ChartList/ChartList.listview.test.tsx | 5 +
.../pages/ChartList/ChartList.permissions.test.tsx | 50 +++++++-
superset-frontend/src/pages/ChartList/index.tsx | 88 ++++++-------
.../DashboardList/DashboardList.behavior.test.tsx | 5 +
.../src/pages/DashboardList/DashboardList.test.tsx | 5 +
.../src/pages/DashboardList/index.tsx | 81 ++++++------
superset-frontend/src/pages/DatabaseList/index.tsx | 98 ++++++---------
.../DatasetList/DatasetList.behavior.test.tsx | 5 +
.../DatasetList/DatasetList.listview.test.tsx | 51 +++-----
.../pages/DatasetList/DatasetList.testHelpers.tsx | 5 +
superset-frontend/src/pages/DatasetList/index.tsx | 138 ++++++++-------------
superset-frontend/src/pages/Home/index.tsx | 4 +-
superset-frontend/src/views/CRUD/types.ts | 4 +-
superset-frontend/src/views/CRUD/utils.tsx | 17 +--
26 files changed, 653 insertions(+), 355 deletions(-)
diff --git a/superset-frontend/spec/helpers/mockBootstrapData.ts b/superset-frontend/spec/helpers/mockBootstrapData.ts
new file mode 100644
index 00000000000..309e1fef0ff
--- /dev/null
+++ b/superset-frontend/spec/helpers/mockBootstrapData.ts
@@ -0,0 +1,47 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Shared `jest.mock('src/utils/getBootstrapData')` factory for list view
+ * tests that rely on `isUserEditorOrAdmin`'s subject-based check
+ * (`common.user_subjects`) to treat a mock user as a subject editor.
+ *
+ * Usage:
+ * jest.mock('src/utils/getBootstrapData', () =>
+ * mockUserSubjectsBootstrapData([1]),
+ * );
+ *
+ * NOTE: the `mock` name prefix is required so babel-plugin-jest-hoist allows
+ * referencing this imported function from inside a `jest.mock()` factory.
+ */
+export const mockUserSubjectsBootstrapData = (userSubjects: number[]) => {
+ const actual = jest.requireActual('src/utils/getBootstrapData');
+ const { DEFAULT_BOOTSTRAP_DATA } = jest.requireActual('src/constants');
+ return {
+ __esModule: true,
+ ...actual,
+ default: jest.fn(() => ({
+ ...DEFAULT_BOOTSTRAP_DATA,
+ common: {
+ ...DEFAULT_BOOTSTRAP_DATA.common,
+ user_subjects: userSubjects,
+ },
+ })),
+ };
+};
diff --git a/superset-frontend/src/dashboard/components/IconButton.test.tsx b/superset-frontend/src/dashboard/components/IconButton.test.tsx
new file mode 100644
index 00000000000..289cafd4a74
--- /dev/null
+++ b/superset-frontend/src/dashboard/components/IconButton.test.tsx
@@ -0,0 +1,75 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { render, screen, fireEvent } from 'spec/helpers/testing-library';
+import IconButton from './IconButton';
+
+const icon = <span data-test="fake-icon">icon</span>;
+
+test('calls onClick when clicked and not disabled', () => {
+ const onClick = jest.fn();
+ render(<IconButton icon={icon} onClick={onClick} data-test="my-button" />);
+
+ fireEvent.click(screen.getByTestId('my-button'));
+
+ expect(onClick).toHaveBeenCalledTimes(1);
+ // aria-disabled is only rendered when the `disabled` prop is set
+ expect(screen.getByTestId('my-button')).not.toHaveAttribute(
+ 'aria-disabled',
+ 'true',
+ );
+});
+
+test('does not call onClick and sets aria-disabled when disabled', () => {
+ const onClick = jest.fn();
+ render(
+ <IconButton icon={icon} onClick={onClick} disabled data-test="my-button" />,
+ );
+
+ const button = screen.getByTestId('my-button');
+ fireEvent.click(button);
+
+ expect(onClick).not.toHaveBeenCalled();
+ expect(button).toHaveAttribute('aria-disabled', 'true');
+});
+
+test('does not prevent default keyboard navigation', () => {
+ const onKeyDown = jest.fn();
+ render(
+ <IconButton
+ icon={icon}
+ onClick={jest.fn()}
+ onKeyDown={onKeyDown}
+ data-test="my-button"
+ />,
+ );
+
+ const eventWasNotCancelled = fireEvent.keyDown(
+ screen.getByTestId('my-button'),
+ { key: 'Tab' },
+ );
+
+ expect(eventWasNotCancelled).toBe(true);
+ expect(onKeyDown).toHaveBeenCalledTimes(1);
+});
+
+test('renders the provided label', () => {
+ render(<IconButton icon={icon} onClick={jest.fn()} label="My Label" />);
+
+ expect(screen.getByText('My Label')).toBeInTheDocument();
+});
diff --git a/superset-frontend/src/dashboard/components/IconButton.tsx b/superset-frontend/src/dashboard/components/IconButton.tsx
index a70d7c3d217..d497f603a6b 100644
--- a/superset-frontend/src/dashboard/components/IconButton.tsx
+++ b/superset-frontend/src/dashboard/components/IconButton.tsx
@@ -16,41 +16,81 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { MouseEventHandler } from 'react';
-import { styled } from '@apache-superset/core/theme';
+import { forwardRef, HTMLAttributes, MouseEventHandler } from 'react';
+import { styled, SupersetTheme } from '@apache-superset/core/theme';
-interface IconButtonProps {
+interface IconButtonProps extends HTMLAttributes<HTMLDivElement> {
icon: JSX.Element;
label?: string;
onClick: MouseEventHandler<HTMLDivElement>;
+ disabled?: boolean;
+ 'data-test'?: string;
}
-const StyledDiv = styled.div`
+const disabledCss = `
+ cursor: not-allowed;
+ opacity: 0.5;
+`;
+
+const activeCss = ({ theme }: { theme: SupersetTheme }) => `
+ &:hover {
+ color: ${theme.colorPrimary};
+ background: ${theme.colorBgTextHover};
+ }
+`;
+
+const StyledDiv = styled.div<{ isDisabled?: boolean }>`
display: flex;
align-items: center;
cursor: pointer;
color: ${({ theme }) => theme.colorIcon};
- &:hover {
- color: ${({ theme }) => theme.colorPrimary};
- }
+ padding: ${({ theme }) => theme.paddingXXS}px;
+ border-radius: ${({ theme }) => theme.borderRadiusXS}px;
+
+ ${({ isDisabled, theme }) => (isDisabled ? disabledCss : activeCss({ theme }))}
`;
const StyledSpan = styled.span`
margin-left: ${({ theme }) => theme.sizeUnit * 2}px;
`;
-const IconButton = ({ icon, label, onClick }: IconButtonProps) => (
- <StyledDiv
- tabIndex={0}
- role="button"
- onClick={e => {
- e.preventDefault();
- onClick(e);
- }}
- >
- {icon}
- {label && <StyledSpan>{label}</StyledSpan>}
- </StyledDiv>
+const IconButton = forwardRef<HTMLDivElement, IconButtonProps>(
+ (
+ {
+ icon,
+ label,
+ onClick,
+ onKeyDown,
+ disabled,
+ 'data-test': dataTest,
+ ...rest
+ },
+ ref,
+ ) => (
+ <StyledDiv
+ {...rest}
+ ref={ref}
+ tabIndex={disabled ? -1 : 0}
+ role="button"
+ isDisabled={disabled}
+ aria-disabled={disabled}
+ data-test={dataTest}
+ onClick={e => {
+ e.preventDefault();
+ if (!disabled) {
+ onClick(e);
+ }
+ }}
+ onKeyDown={e => {
+ if (!disabled) {
+ onKeyDown?.(e);
+ }
+ }}
+ >
+ {icon}
+ {label && <StyledSpan>{label}</StyledSpan>}
+ </StyledDiv>
+ ),
);
export default IconButton;
diff --git a/superset-frontend/src/dashboard/util/permissionUtils.test.ts b/superset-frontend/src/dashboard/util/permissionUtils.test.ts
index badd5da5cb3..fa548403188 100644
--- a/superset-frontend/src/dashboard/util/permissionUtils.test.ts
+++ b/superset-frontend/src/dashboard/util/permissionUtils.test.ts
@@ -29,6 +29,7 @@ import {
canUserSaveAsDashboard,
isUserAdmin,
isUserDashboardEditor,
+ isUserEditorOrAdmin,
} from './permissionUtils';
const editorUser: UserWithPermissionsAndRoles = {
@@ -196,6 +197,32 @@ test('isUserAdmin returns false for non-admin user', () => {
expect(isUserAdmin(editorUser)).toEqual(false);
});
+describe('isUserEditorOrAdmin', () => {
+ test('returns true when the user is a subject editor', () => {
+ expect(isUserEditorOrAdmin(editorUser, [editorSubject])).toEqual(true);
+ });
+
+ test('returns true when the user is an admin, regardless of subject membership', () => {
+ const nonMatchingSubject: Subject = { id: 999, label: 'Other', type: 1 };
+ expect(isUserEditorOrAdmin(adminUser, [nonMatchingSubject])).toEqual(true);
+ });
+
+ test('returns false when the user is neither a subject editor nor an admin', () => {
+ const nonMatchingSubject: Subject = { id: 999, label: 'Other', type: 1 };
+ expect(isUserEditorOrAdmin(outsiderUser, [nonMatchingSubject])).toEqual(
+ false,
+ );
+ });
+
+ test('returns false when editors is empty', () => {
+ expect(isUserEditorOrAdmin(editorUser, [])).toEqual(false);
+ });
+
+ test('returns false when editors is omitted', () => {
+ expect(isUserEditorOrAdmin(outsiderUser)).toEqual(false);
+ });
+});
+
test('userHasPermission always returns true for admin user', () => {
arbitraryPermissions.forEach(permissionView => {
expect(
diff --git a/superset-frontend/src/dashboard/util/permissionUtils.ts b/superset-frontend/src/dashboard/util/permissionUtils.ts
index 7da82c3c9bb..3c74b901f40 100644
--- a/superset-frontend/src/dashboard/util/permissionUtils.ts
+++ b/superset-frontend/src/dashboard/util/permissionUtils.ts
@@ -22,6 +22,7 @@ import {
UserWithPermissionsAndRoles,
} from 'src/types/bootstrapTypes';
import { Dashboard } from 'src/types/Dashboard';
+import Subject from 'src/types/Subject';
import { findPermission } from 'src/utils/findPermission';
import getBootstrapData from 'src/utils/getBootstrapData';
@@ -32,6 +33,11 @@ const ADMIN_ROLE_NAME = 'admin';
const getUserSubjects = (): number[] =>
getBootstrapData()?.common?.user_subjects ?? [];
+const isUserInEditors = (editors: Subject[] = []): boolean => {
+ const userSubjects = getUserSubjects();
+ return editors.some(editor => userSubjects.includes(editor.id));
+};
+
export const isUserAdmin = (
user?: UserWithPermissionsAndRoles | UndefinedUser,
) =>
@@ -40,12 +46,13 @@ export const isUserAdmin = (
role => role.toLowerCase() === ADMIN_ROLE_NAME,
);
-export const isUserDashboardEditor = (dashboard: Dashboard): boolean => {
- const userSubjects = getUserSubjects();
- return (
- dashboard.editors?.some(editor => userSubjects.includes(editor.id)) ?? false
- );
-};
+export const isUserEditorOrAdmin = (
+ user?: UserWithPermissionsAndRoles | UndefinedUser,
+ editors: Subject[] = [],
+): boolean => isUserInEditors(editors) || isUserAdmin(user);
+
+export const isUserDashboardEditor = (dashboard: Dashboard): boolean =>
+ isUserInEditors(dashboard.editors);
export const canUserEditDashboard = (
dashboard: Dashboard,
diff --git a/superset-frontend/src/features/charts/ChartCard.tsx b/superset-frontend/src/features/charts/ChartCard.tsx
index b55904f7144..1be45a91aa4 100644
--- a/superset-frontend/src/features/charts/ChartCard.tsx
+++ b/superset-frontend/src/features/charts/ChartCard.tsx
@@ -31,6 +31,7 @@ import {
Label,
ListViewCard,
MenuItem,
+ Tooltip,
} from '@superset-ui/core/components';
import Chart from 'src/types/Chart';
import { SubjectPile } from 'src/features/subjects/SubjectPile';
@@ -39,6 +40,8 @@ import { handleChartDelete, CardStyles } from 'src/views/CRUD/utils';
import { assetUrl } from 'src/utils/assetUrl';
import type { ListViewFetchDataConfig as FetchDataConfig } from 'src/components';
import { TableTab } from 'src/views/CRUD/types';
+import { isUserEditorOrAdmin } from 'src/dashboard/util/permissionUtils';
+import type { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
interface ChartCardProps {
chart: Chart;
@@ -52,7 +55,7 @@ interface ChartCardProps {
saveFavoriteStatus: (id: number, isStarred: boolean) => void;
favoriteStatus: boolean;
chartFilter?: string;
- userId?: string | number;
+ user?: UserWithPermissionsAndRoles;
showThumbnails?: boolean;
handleBulkChartExport: (chartsToExport: Chart[]) => void;
getData?: (tab: TableTab) => void;
@@ -71,36 +74,54 @@ export default function ChartCard({
saveFavoriteStatus,
favoriteStatus,
chartFilter,
- userId,
+ user,
handleBulkChartExport,
getData,
}: ChartCardProps) {
+ const userId = user?.userId;
+
const history = useHistory();
const canEdit = hasPerm('can_write');
const canDelete = hasPerm('can_write');
const canExport = hasPerm('can_export');
+ const allowEdit = isUserEditorOrAdmin(user, chart.editors);
const menuItems: MenuItem[] = [];
if (canEdit) {
menuItems.push({
key: 'edit',
label: (
- <div
- data-test="chart-list-edit-option"
- role="button"
- tabIndex={0}
- onClick={() => openChartEditModal(chart)}
- onKeyDown={handleKeyboardActivation(() => openChartEditModal(chart))}
+ <Tooltip
+ title={
+ allowEdit
+ ? null
+ : t(
+ 'You must be a chart editor in order to edit. Please reach out to a chart editor to request modifications or edit access.',
+ )
+ }
>
- <Icons.EditOutlined
- iconSize="l"
- css={css`
- vertical-align: text-top;
- `}
- />{' '}
- {t('Edit')}
- </div>
+ <div
+ data-test="chart-list-edit-option"
+ role="button"
+ tabIndex={0}
+ onClick={allowEdit ? () => openChartEditModal(chart) : undefined}
+ onKeyDown={
+ allowEdit
+ ? handleKeyboardActivation(() => openChartEditModal(chart))
+ : undefined
+ }
+ >
+ <Icons.EditOutlined
+ iconSize="l"
+ css={css`
+ vertical-align: text-top;
+ `}
+ />{' '}
+ {t('Edit')}
+ </div>
+ </Tooltip>
),
+ disabled: !allowEdit,
});
}
@@ -152,25 +173,40 @@ export default function ChartCard({
}
>
{confirmDelete => (
- <div
- data-test="chart-list-delete-option"
- role="button"
- tabIndex={0}
- className="action-button"
- onClick={confirmDelete}
- onKeyDown={handleKeyboardActivation(confirmDelete)}
+ <Tooltip
+ title={
+ allowEdit
+ ? null
+ : t(
+ 'You must be a chart editor in order to delete. Please reach out to a chart editor to request modifications or edit access.',
+ )
+ }
>
- <Icons.DeleteOutlined
- iconSize="l"
- css={css`
- vertical-align: text-top;
- `}
- />{' '}
- {t('Delete')}
- </div>
+ <div
+ data-test="chart-list-delete-option"
+ role="button"
+ tabIndex={0}
+ className="action-button"
+ onClick={allowEdit ? confirmDelete : undefined}
+ onKeyDown={
+ allowEdit
+ ? handleKeyboardActivation(confirmDelete)
+ : undefined
+ }
+ >
+ <Icons.DeleteOutlined
+ iconSize="l"
+ css={css`
+ vertical-align: text-top;
+ `}
+ />{' '}
+ {t('Delete')}
+ </div>
+ </Tooltip>
)}
</ConfirmStatusChange>
),
+ disabled: !allowEdit,
});
}
diff --git a/superset-frontend/src/features/dashboards/DashboardCard.tsx b/superset-frontend/src/features/dashboards/DashboardCard.tsx
index 1b1096a24dc..b40542cac40 100644
--- a/superset-frontend/src/features/dashboards/DashboardCard.tsx
+++ b/superset-frontend/src/features/dashboards/DashboardCard.tsx
@@ -29,12 +29,15 @@ import {
Icons,
PublishedLabel,
ListViewCard,
+ Tooltip,
} from '@superset-ui/core/components';
import { MenuItem } from '@superset-ui/core/components/Menu';
import { Dashboard } from 'src/views/CRUD/types';
import { assetUrl } from 'src/utils/assetUrl';
import { SubjectPile } from 'src/features/subjects/SubjectPile';
import { KebabMenuButton } from 'src/components';
+import { isUserEditorOrAdmin } from 'src/dashboard/util/permissionUtils';
+import type { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
interface DashboardCardProps {
isChart?: boolean;
@@ -45,7 +48,7 @@ interface DashboardCardProps {
openDashboardEditModal?: (d: Dashboard) => void;
saveFavoriteStatus: (id: number, isStarred: boolean) => void;
favoriteStatus: boolean;
- userId?: string | number;
+ user?: UserWithPermissionsAndRoles;
showThumbnails?: boolean;
handleBulkDashboardExport: (dashboardsToExport: Dashboard[]) => void;
onDelete: (dashboard: Dashboard) => void;
@@ -55,7 +58,7 @@ function DashboardCard({
dashboard,
hasPerm,
bulkSelectEnabled,
- userId,
+ user,
openDashboardEditModal,
favoriteStatus,
saveFavoriteStatus,
@@ -63,10 +66,13 @@ function DashboardCard({
handleBulkDashboardExport,
onDelete,
}: DashboardCardProps) {
+ const userId = user?.userId;
+
const history = useHistory();
const canEdit = hasPerm('can_write');
const canDelete = hasPerm('can_write');
const canExport = hasPerm('can_export');
+ const allowEdit = isUserEditorOrAdmin(user, dashboard.editors);
const digest = dashboard.changed_on_utc || dashboard.changed_on;
const thumbnailUrl =
isFeatureEnabled(FeatureFlag.Thumbnails) && dashboard.id && digest
@@ -79,19 +85,36 @@ function DashboardCard({
menuItems.push({
key: 'edit',
label: (
- <div
- role="button"
- tabIndex={0}
- className="action-button"
- onClick={() => openDashboardEditModal(dashboard)}
- onKeyDown={handleKeyboardActivation(() =>
- openDashboardEditModal(dashboard),
- )}
- data-test="dashboard-card-option-edit-button"
+ <Tooltip
+ title={
+ allowEdit
+ ? null
+ : t(
+ 'You must be a dashboard editor in order to edit. Please reach out to a dashboard editor to request modifications or edit access.',
+ )
+ }
>
- <Icons.EditOutlined iconSize="l" data-test="edit-alt" /> {t('Edit')}
- </div>
+ <div
+ role="button"
+ tabIndex={0}
+ className="action-button"
+ onClick={
+ allowEdit ? () => openDashboardEditModal(dashboard) : undefined
+ }
+ onKeyDown={
+ allowEdit
+ ? handleKeyboardActivation(() =>
+ openDashboardEditModal(dashboard),
+ )
+ : undefined
+ }
+ data-test="dashboard-card-option-edit-button"
+ >
+ <Icons.EditOutlined iconSize="l" data-test="edit-alt" /> {t('Edit')}
+ </div>
+ </Tooltip>
),
+ disabled: !allowEdit,
});
}
@@ -119,17 +142,32 @@ function DashboardCard({
menuItems.push({
key: 'delete',
label: (
- <div
- role="button"
- tabIndex={0}
- className="action-button"
- onClick={() => onDelete(dashboard)}
- onKeyDown={handleKeyboardActivation(() => onDelete(dashboard))}
- data-test="dashboard-card-option-delete-button"
+ <Tooltip
+ title={
+ allowEdit
+ ? null
+ : t(
+ 'You must be a dashboard editor in order to delete. Please reach out to a dashboard editor to request modifications or edit access.',
+ )
+ }
>
- <Icons.DeleteOutlined iconSize="l" /> {t('Delete')}
- </div>
+ <div
+ role="button"
+ tabIndex={0}
+ className="action-button"
+ onClick={allowEdit ? () => onDelete(dashboard) : undefined}
+ onKeyDown={
+ allowEdit
+ ? handleKeyboardActivation(() => onDelete(dashboard))
+ : undefined
+ }
+ data-test="dashboard-card-option-delete-button"
+ >
+ <Icons.DeleteOutlined iconSize="l" /> {t('Delete')}
+ </div>
+ </Tooltip>
),
+ disabled: !allowEdit,
});
}
diff --git a/superset-frontend/src/features/home/ChartTable.test.tsx b/superset-frontend/src/features/home/ChartTable.test.tsx
index fd796ca9818..2122fd3665d 100644
--- a/superset-frontend/src/features/home/ChartTable.test.tsx
+++ b/superset-frontend/src/features/home/ChartTable.test.tsx
@@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
+import { mockUserSubjectsBootstrapData } from 'spec/helpers/mockBootstrapData';
import {
render,
screen,
@@ -35,6 +36,10 @@ jest.mock('src/utils/export', () => ({
default: jest.fn(),
}));
+jest.mock('src/utils/getBootstrapData', () =>
+ mockUserSubjectsBootstrapData([2]),
+);
+
const mockExport = handleResourceExport as jest.MockedFunction<
typeof handleResourceExport
>;
@@ -52,6 +57,7 @@ const mockCharts = Array.from({ length: 3 }).map((_, i) => ({
viz_type: VizType.Bar,
datasource_title: `ds${i}`,
thumbnail_url: '',
+ editors: [{ id: 2, label: 'Admin User', type: 1 }],
}));
fetchMock.get(
diff --git a/superset-frontend/src/features/home/ChartTable.tsx b/superset-frontend/src/features/home/ChartTable.tsx
index 37d594c7c50..2829634c362 100644
--- a/superset-frontend/src/features/home/ChartTable.tsx
+++ b/superset-frontend/src/features/home/ChartTable.tsx
@@ -32,7 +32,7 @@ import withToasts from 'src/components/MessageToasts/withToasts';
import { useHistory } from 'react-router-dom';
import { Filter, TableTab } from 'src/views/CRUD/types';
import PropertiesModal from 'src/explore/components/PropertiesModal';
-import { User } from 'src/types/bootstrapTypes';
+import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
import {
CardContainer,
getFilterValues,
@@ -53,7 +53,7 @@ import SubMenu from './SubMenu';
interface ChartTableProps {
addDangerToast: (message: string) => void;
addSuccessToast: (message: string) => void;
- user?: User;
+ user?: UserWithPermissionsAndRoles;
mine: Array<any>;
showThumbnails: boolean;
otherTabData?: Array<object>;
@@ -228,7 +228,7 @@ function ChartTable({
openChartEditModal={openChartEditModal}
chartFilter={activeTab}
chart={e}
- userId={user?.userId}
+ user={user}
hasPerm={hasPerm}
showThumbnails={showThumbnails}
bulkSelectEnabled={bulkSelectEnabled}
diff --git a/superset-frontend/src/features/home/DashboardTable.test.tsx b/superset-frontend/src/features/home/DashboardTable.test.tsx
index 57d70dc7b15..e08d6b6aad7 100644
--- a/superset-frontend/src/features/home/DashboardTable.test.tsx
+++ b/superset-frontend/src/features/home/DashboardTable.test.tsx
@@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
+import { mockUserSubjectsBootstrapData } from 'spec/helpers/mockBootstrapData';
import {
render,
screen,
@@ -37,6 +38,10 @@ jest.mock('src/utils/export', () => ({
default: jest.fn(),
}));
+jest.mock('src/utils/getBootstrapData', () =>
+ mockUserSubjectsBootstrapData([1]),
+);
+
const mockExport = handleResourceExport as jest.MockedFunction<
typeof handleResourceExport
>;
@@ -78,6 +83,7 @@ const mockDashboards = [
changed_on_delta_humanized: '1 day ago',
url: '/dashboard/1',
thumbnail_url: '/thumbnail/1',
+ editors: [{ id: 1, label: 'Test User', type: 1 }],
},
{
id: 2,
@@ -85,6 +91,7 @@ const mockDashboards = [
changed_on_delta_humanized: '2 days ago',
url: '/dashboard/2',
thumbnail_url: '/thumbnail/2',
+ editors: [{ id: 1, label: 'Test User', type: 1 }],
},
];
diff --git a/superset-frontend/src/features/home/DashboardTable.tsx b/superset-frontend/src/features/home/DashboardTable.tsx
index c8987fc4ef2..9d6d04d8b3c 100644
--- a/superset-frontend/src/features/home/DashboardTable.tsx
+++ b/superset-frontend/src/features/home/DashboardTable.tsx
@@ -263,7 +263,7 @@ function DashboardTable({
hasPerm={hasPerm}
bulkSelectEnabled={false}
showThumbnails={showThumbnails}
- userId={user?.userId}
+ user={user}
loading={loading}
openDashboardEditModal={(dashboard: Dashboard) =>
setEditModal(dashboard)
diff --git a/superset-frontend/src/pages/ChartList/ChartList.cardview.test.tsx b/superset-frontend/src/pages/ChartList/ChartList.cardview.test.tsx
index b5ba19e2b8d..6a88a55a132 100644
--- a/superset-frontend/src/pages/ChartList/ChartList.cardview.test.tsx
+++ b/superset-frontend/src/pages/ChartList/ChartList.cardview.test.tsx
@@ -17,6 +17,7 @@
* under the License.
*/
import fetchMock from 'fetch-mock';
+import { mockUserSubjectsBootstrapData } from 'spec/helpers/mockBootstrapData';
import {
fireEvent,
screen,
@@ -45,6 +46,10 @@ jest.mock('src/utils/export', () => ({
default: jest.fn(),
}));
+jest.mock('src/utils/getBootstrapData', () =>
+ mockUserSubjectsBootstrapData([1]),
+);
+
const mockUser = {
userId: 1,
firstName: 'Test',
diff --git a/superset-frontend/src/pages/ChartList/ChartList.listview.test.tsx b/superset-frontend/src/pages/ChartList/ChartList.listview.test.tsx
index 8be5c89e767..38427460148 100644
--- a/superset-frontend/src/pages/ChartList/ChartList.listview.test.tsx
+++ b/superset-frontend/src/pages/ChartList/ChartList.listview.test.tsx
@@ -17,6 +17,7 @@
* under the License.
*/
import fetchMock from 'fetch-mock';
+import { mockUserSubjectsBootstrapData } from 'spec/helpers/mockBootstrapData';
import { screen, waitFor, within } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import { isFeatureEnabled } from '@superset-ui/core';
@@ -43,6 +44,10 @@ jest.mock('src/utils/export', () => ({
default: jest.fn(),
}));
+jest.mock('src/utils/getBootstrapData', () =>
+ mockUserSubjectsBootstrapData([1]),
+);
+
const mockIsFeatureEnabled = isFeatureEnabled as jest.MockedFunction<
typeof isFeatureEnabled
>;
diff --git a/superset-frontend/src/pages/ChartList/ChartList.permissions.test.tsx b/superset-frontend/src/pages/ChartList/ChartList.permissions.test.tsx
index 4cb84d1b2ca..0a137f581bd 100644
--- a/superset-frontend/src/pages/ChartList/ChartList.permissions.test.tsx
+++ b/superset-frontend/src/pages/ChartList/ChartList.permissions.test.tsx
@@ -17,7 +17,13 @@
* under the License.
*/
import fetchMock from 'fetch-mock';
-import { render, screen, waitFor } from 'spec/helpers/testing-library';
+import {
+ fireEvent,
+ render,
+ screen,
+ waitFor,
+ within,
+} from 'spec/helpers/testing-library';
import { Provider } from 'react-redux';
import { MemoryRouter } from 'react-router-dom';
import { configureStore } from '@reduxjs/toolkit';
@@ -52,10 +58,19 @@ const PERMISSIONS = {
NONE: [],
};
+const getRoleName = (
+ permissions: (typeof PERMISSIONS)[keyof typeof PERMISSIONS],
+) => (permissions === PERMISSIONS.ADMIN ? 'Admin' : 'TestRole');
+
const createMockUser = (overrides = {}) => ({
userId: 1,
firstName: 'Test',
lastName: 'User',
+ username: 'testuser',
+ permissions: {
+ database_access: [],
+ datasource_access: [],
+ },
roles: {
Admin: [
['can_sqllab', 'Superset'],
@@ -88,7 +103,7 @@ const createStoreStateWithPermissions = (
user: userId
? {
...createMockUser({ userId }),
- roles: { TestRole: permissions },
+ roles: { [getRoleName(permissions)]: permissions },
}
: {},
common: {
@@ -150,7 +165,7 @@ const renderWithPermissions = async (
? {
user: {
...createMockUser({ userId }),
- roles: { TestRole: permissions },
+ roles: { [getRoleName(permissions)]: permissions },
},
}
: { user: { userId: undefined } }; // Explicitly set userId to undefined for logged-out state
@@ -227,6 +242,35 @@ describe('ChartList - Permission-based UI Tests', () => {
expect(deleteButtons).toHaveLength(mockCharts.length);
});
+ test('enables table actions for an admin who is not a chart editor', async () => {
+ await renderWithPermissions(PERMISSIONS.ADMIN);
+ await screen.findByTestId('chart-list-view');
+
+ const row = (await screen.findByText(mockCharts[1].slice_name)).closest(
+ 'tr',
+ );
+ const editButton = within(row!).getByTestId('chart-row-edit');
+ const deleteButton = within(row!).getByTestId('chart-row-delete');
+
+ expect(editButton).toHaveAttribute('aria-disabled', 'false');
+ expect(deleteButton).toHaveAttribute('aria-disabled', 'false');
+ });
+
+ test('enables card actions for an admin who is not a chart editor', async () => {
+ await renderWithPermissions(PERMISSIONS.ADMIN, 1, { cardView: true });
+ await screen.findByTestId('chart-list-view');
+
+ const cardMenus = await screen.findAllByTestId('chart-card-menu');
+ fireEvent.click(cardMenus[1]);
+
+ expect(
+ (await screen.findByText('Edit')).closest('[role="menuitem"]'),
+ ).toHaveAttribute('aria-disabled', 'false');
+ expect(
+ screen.getByText('Delete').closest('[role="menuitem"]'),
+ ).toHaveAttribute('aria-disabled', 'false');
+ });
+
test('hides Actions column for users with read-only permissions', async () => {
await renderWithPermissions(PERMISSIONS.READ_ONLY);
await screen.findByTestId('chart-list-view');
diff --git a/superset-frontend/src/pages/ChartList/index.tsx b/superset-frontend/src/pages/ChartList/index.tsx
index 00ea34af07e..2700617d388 100644
--- a/superset-frontend/src/pages/ChartList/index.tsx
+++ b/superset-frontend/src/pages/ChartList/index.tsx
@@ -86,6 +86,9 @@ import { QueryObjectColumns } from 'src/views/CRUD/types';
import { WIDER_DROPDOWN_WIDTH } from 'src/components/ListView/utils';
import { Tag } from 'src/components/Tag';
import { datasetLabel } from 'src/features/semanticLayers/label';
+import { isUserEditorOrAdmin } from 'src/dashboard/util/permissionUtils';
+import IconButton from 'src/dashboard/components/IconButton';
+import type { CellProps } from 'react-table';
const FlexRowContainer = styled.div`
align-items: center;
@@ -160,23 +163,16 @@ const createFetchDatasets = async (
interface ChartListProps {
addDangerToast: (msg: string) => void;
addSuccessToast: (msg: string) => void;
- user: {
- userId: string | number;
- firstName: string;
- lastName: string;
- };
+ user?: UserWithPermissionsAndRoles;
}
-const StyledActions = styled.div`
+const Actions = styled.div`
color: ${({ theme }) => theme.colorIcon};
`;
function ChartList(props: ChartListProps) {
- const {
- addDangerToast,
- addSuccessToast,
- user: { userId },
- } = props;
+ const { addDangerToast, addSuccessToast, user } = props;
+ const userId = user?.userId;
const history = useHistory();
@@ -229,9 +225,11 @@ function ChartList(props: ChartListProps) {
// TODO: Fix usage of localStorage keying on the user id
const userSettings = useMemo(
() =>
- dangerouslyGetItemDoNotUse(userId?.toString(), null) as {
- thumbnails: boolean;
- },
+ userId === undefined
+ ? null
+ : (dangerouslyGetItemDoNotUse(userId.toString(), null) as {
+ thumbnails: boolean;
+ }),
[userId],
);
@@ -517,7 +515,8 @@ function ChartList(props: ChartListProps) {
id: 'changed_on_delta_humanized',
},
{
- Cell: ({ row: { original } }: any) => {
+ Cell: ({ row: { original } }: CellProps<Chart>) => {
+ const allowEdit = isUserEditorOrAdmin(user, original.editors);
const handleDelete = () =>
handleChartDelete(
original,
@@ -532,23 +531,28 @@ function ChartList(props: ChartListProps) {
}
return (
- <StyledActions className="actions">
+ <Actions className="actions">
{canEdit && (
<Tooltip
id="edit-action-tooltip"
- title={t('Edit')}
+ title={
+ allowEdit
+ ? t('Edit')
+ : t(
+ 'You must be a chart editor in order to edit. Please reach out to a chart editor to request modifications or edit access.',
+ )
+ }
placement="bottom"
>
- <span
+ <IconButton
data-test="chart-row-edit"
- role="button"
- tabIndex={0}
- className="action-button"
+ disabled={!allowEdit}
onClick={openEditModal}
onKeyDown={handleKeyboardActivation(openEditModal)}
- >
- <Icons.EditOutlined data-test="edit-alt" iconSize="l" />
- </span>
+ icon={
+ <Icons.EditOutlined data-test="edit-alt" iconSize="l" />
+ }
+ />
</Tooltip>
)}
{canExport && (
@@ -557,16 +561,12 @@ function ChartList(props: ChartListProps) {
title={t('Export')}
placement="bottom"
>
- <span
+ <IconButton
data-test="chart-row-export"
- role="button"
- tabIndex={0}
- className="action-button"
onClick={handleExport}
onKeyDown={handleKeyboardActivation(handleExport)}
- >
- <Icons.UploadOutlined iconSize="l" />
- </span>
+ icon={<Icons.UploadOutlined iconSize="l" />}
+ />
</Tooltip>
)}
{canDelete && (
@@ -583,24 +583,27 @@ function ChartList(props: ChartListProps) {
{confirmDelete => (
<Tooltip
id="delete-action-tooltip"
- title={t('Delete')}
+ title={
+ allowEdit
+ ? t('Delete')
+ : t(
+ 'You must be a chart editor in order to delete. Please reach out to a chart editor to request modifications or edit access.',
+ )
+ }
placement="bottom"
>
- <span
+ <IconButton
data-test="chart-row-delete"
- role="button"
- tabIndex={0}
- className="action-button"
+ disabled={!allowEdit}
onClick={confirmDelete}
onKeyDown={handleKeyboardActivation(confirmDelete)}
- >
- <Icons.DeleteOutlined iconSize="l" />
- </span>
+ icon={<Icons.DeleteOutlined iconSize="l" />}
+ />
</Tooltip>
)}
</ConfirmStatusChange>
)}
- </StyledActions>
+ </Actions>
);
},
Header: t('Actions'),
@@ -616,6 +619,7 @@ function ChartList(props: ChartListProps) {
},
],
[
+ user,
userId,
canEdit,
canDelete,
@@ -849,7 +853,7 @@ function ChartList(props: ChartListProps) {
addDangerToast={addDangerToast}
addSuccessToast={addSuccessToast}
refreshData={refreshData}
- userId={userId}
+ user={user}
loading={loading}
favoriteStatus={favoriteStatus[chart.id]}
saveFavoriteStatus={saveFavoriteStatus}
@@ -867,7 +871,7 @@ function ChartList(props: ChartListProps) {
openChartEditModal,
refreshData,
saveFavoriteStatus,
- userId,
+ user,
userSettings,
],
);
diff --git a/superset-frontend/src/pages/DashboardList/DashboardList.behavior.test.tsx b/superset-frontend/src/pages/DashboardList/DashboardList.behavior.test.tsx
index 08d0547a46c..64c26c13f8b 100644
--- a/superset-frontend/src/pages/DashboardList/DashboardList.behavior.test.tsx
+++ b/superset-frontend/src/pages/DashboardList/DashboardList.behavior.test.tsx
@@ -17,6 +17,7 @@
* under the License.
*/
import fetchMock from 'fetch-mock';
+import { mockUserSubjectsBootstrapData } from 'spec/helpers/mockBootstrapData';
import { fireEvent, screen, waitFor } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import { isFeatureEnabled } from '@superset-ui/core';
@@ -39,6 +40,10 @@ jest.mock('src/utils/export', () => ({
default: jest.fn(),
}));
+jest.mock('src/utils/getBootstrapData', () =>
+ mockUserSubjectsBootstrapData([1]),
+);
+
const mockIsFeatureEnabled = isFeatureEnabled as jest.MockedFunction<
typeof isFeatureEnabled
>;
diff --git a/superset-frontend/src/pages/DashboardList/DashboardList.test.tsx b/superset-frontend/src/pages/DashboardList/DashboardList.test.tsx
index ad59b23f5cd..b43607d4f68 100644
--- a/superset-frontend/src/pages/DashboardList/DashboardList.test.tsx
+++ b/superset-frontend/src/pages/DashboardList/DashboardList.test.tsx
@@ -18,6 +18,7 @@
*/
import fetchMock from 'fetch-mock';
import { isFeatureEnabled } from '@superset-ui/core';
+import { mockUserSubjectsBootstrapData } from 'spec/helpers/mockBootstrapData';
import {
screen,
selectPillOption,
@@ -45,6 +46,10 @@ jest.mock('src/utils/export', () => ({
default: jest.fn(),
}));
+jest.mock('src/utils/getBootstrapData', () =>
+ mockUserSubjectsBootstrapData([1]),
+);
+
const mockIsFeatureEnabled = isFeatureEnabled as jest.MockedFunction<
typeof isFeatureEnabled
>;
diff --git a/superset-frontend/src/pages/DashboardList/index.tsx b/superset-frontend/src/pages/DashboardList/index.tsx
index 91d3ef3bc80..b2f475f4c89 100644
--- a/superset-frontend/src/pages/DashboardList/index.tsx
+++ b/superset-frontend/src/pages/DashboardList/index.tsx
@@ -80,6 +80,9 @@ import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
import { findPermission } from 'src/utils/findPermission';
import { navigateTo } from 'src/utils/navigationUtils';
import { WIDER_DROPDOWN_WIDTH } from 'src/components/ListView/utils';
+import { isUserEditorOrAdmin } from 'src/dashboard/util/permissionUtils';
+import IconButton from 'src/dashboard/components/IconButton';
+import type { CellProps } from 'react-table';
const PAGE_SIZE = 25;
const PASSWORDS_NEEDED_MESSAGE = t(
@@ -98,11 +101,7 @@ const CONFIRM_OVERWRITE_MESSAGE = t(
interface DashboardListProps {
addDangerToast: (msg: string) => void;
addSuccessToast: (msg: string) => void;
- user: {
- userId: string | number;
- firstName: string;
- lastName: string;
- };
+ user?: UserWithPermissionsAndRoles;
}
export interface Dashboard {
@@ -244,7 +243,10 @@ function DashboardList(props: DashboardListProps) {
};
// TODO: Fix usage of localStorage keying on the user id
- const userKey = dangerouslyGetItemDoNotUse(user?.userId?.toString(), null);
+ const userKey =
+ user?.userId === undefined
+ ? null
+ : dangerouslyGetItemDoNotUse(user.userId.toString(), null);
const canCreate = hasPerm('can_write');
const canEdit = hasPerm('can_write');
@@ -479,7 +481,8 @@ function DashboardList(props: DashboardListProps) {
id: 'changed_on_delta_humanized',
},
{
- Cell: ({ row: { original } }: any) => {
+ Cell: ({ row: { original } }: CellProps<Dashboard>) => {
+ const allowEdit = isUserEditorOrAdmin(user, original.editors);
const handleDelete = () =>
handleDashboardDelete(
original,
@@ -495,19 +498,24 @@ function DashboardList(props: DashboardListProps) {
{canEdit && (
<Tooltip
id="edit-action-tooltip"
- title={t('Edit')}
+ title={
+ allowEdit
+ ? t('Edit')
+ : t(
+ 'You must be a dashboard editor in order to edit. Please reach out to a dashboard editor to request modifications or edit access.',
+ )
+ }
placement="bottom"
>
- <span
+ <IconButton
data-test="dashboard-row-edit"
- role="button"
- tabIndex={0}
- className="action-button"
+ disabled={!allowEdit}
onClick={handleEdit}
onKeyDown={handleKeyboardActivation(handleEdit)}
- >
- <Icons.EditOutlined data-test="edit-alt" iconSize="l" />
- </span>
+ icon={
+ <Icons.EditOutlined data-test="edit-alt" iconSize="l" />
+ }
+ />
</Tooltip>
)}
{canExport && (
@@ -516,16 +524,12 @@ function DashboardList(props: DashboardListProps) {
title={t('Export')}
placement="bottom"
>
- <span
+ <IconButton
data-test="dashboard-row-export"
- role="button"
- tabIndex={0}
- className="action-button"
onClick={handleExport}
onKeyDown={handleKeyboardActivation(handleExport)}
- >
- <Icons.UploadOutlined iconSize="l" />
- </span>
+ icon={<Icons.UploadOutlined iconSize="l" />}
+ />
</Tooltip>
)}
{canDelete && (
@@ -542,22 +546,27 @@ function DashboardList(props: DashboardListProps) {
{confirmDelete => (
<Tooltip
id="delete-action-tooltip"
- title={t('Delete')}
+ title={
+ allowEdit
+ ? t('Delete')
+ : t(
+ 'You must be a dashboard editor in order to delete. Please reach out to a dashboard editor to request modifications or edit access.',
+ )
+ }
placement="bottom"
>
- <span
+ <IconButton
data-test="dashboard-row-delete"
- role="button"
- tabIndex={0}
- className="action-button"
+ disabled={!allowEdit}
onClick={confirmDelete}
onKeyDown={handleKeyboardActivation(confirmDelete)}
- >
- <Icons.DeleteOutlined
- iconSize="l"
- data-test="dashboard-list-trash-icon"
- />
- </span>
+ icon={
+ <Icons.DeleteOutlined
+ iconSize="l"
+ data-test="dashboard-list-trash-icon"
+ />
+ }
+ />
</Tooltip>
)}
</ConfirmStatusChange>
@@ -577,7 +586,7 @@ function DashboardList(props: DashboardListProps) {
},
],
[
- user?.userId,
+ user,
canEdit,
canDelete,
canExport,
@@ -763,7 +772,7 @@ function DashboardList(props: DashboardListProps) {
? userKey.thumbnails
: isFeatureEnabled(FeatureFlag.Thumbnails)
}
- userId={user?.userId}
+ user={user}
loading={loading}
openDashboardEditModal={openDashboardEditModal}
saveFavoriteStatus={saveFavoriteStatus}
@@ -777,7 +786,7 @@ function DashboardList(props: DashboardListProps) {
favoriteStatus,
hasPerm,
loading,
- user?.userId,
+ user,
saveFavoriteStatus,
userKey,
handleBulkDashboardExport,
diff --git a/superset-frontend/src/pages/DatabaseList/index.tsx b/superset-frontend/src/pages/DatabaseList/index.tsx
index e36c3f38819..cbf4107ee8e 100644
--- a/superset-frontend/src/pages/DatabaseList/index.tsx
+++ b/superset-frontend/src/pages/DatabaseList/index.tsx
@@ -24,7 +24,7 @@ import {
FeatureFlag,
handleKeyboardActivation,
} from '@superset-ui/core';
-import { css, styled, useTheme } from '@apache-superset/core/theme';
+import { css, useTheme } from '@apache-superset/core/theme';
import { useState, useMemo, useEffect, useCallback } from 'react';
import type { CellProps } from 'react-table';
import rison from 'rison';
@@ -77,6 +77,7 @@ import {
databaseLabelLower,
databasesLabel,
} from 'src/features/semanticLayers/label';
+import IconButton from 'src/dashboard/components/IconButton';
const extensionsRegistry = getExtensionsRegistry();
const DatabaseDeleteRelatedExtension = extensionsRegistry.get(
@@ -112,14 +113,6 @@ interface DatabaseListProps {
};
}
-const Actions = styled.div`
- .action-button {
- display: inline-block;
- height: 100%;
- color: ${({ theme }) => theme.colorIcon};
- }
-`;
-
function BooleanDisplay({ value }: { value: boolean }) {
return value ? (
<Icons.CheckOutlined iconSize="s" />
@@ -686,24 +679,20 @@ function DatabaseList({
if (isSemanticLayer) {
if (!canEdit && !canDelete) return null;
return (
- <Actions className="actions">
+ <div className="actions">
{canDelete && (
<Tooltip
id="delete-action-tooltip"
title={t('Delete')}
placement="bottom"
>
- <span
- role="button"
- tabIndex={0}
- className="action-button"
+ <IconButton
onClick={() => setSlCurrentlyDeleting(original)}
onKeyDown={handleKeyboardActivation(() =>
setSlCurrentlyDeleting(original),
)}
- >
- <Icons.DeleteOutlined iconSize="l" />
- </span>
+ icon={<Icons.DeleteOutlined iconSize="l" />}
+ />
</Tooltip>
)}
{canEdit && (
@@ -712,22 +701,18 @@ function DatabaseList({
title={t('Edit')}
placement="bottom"
>
- <span
- role="button"
- tabIndex={0}
- className="action-button"
+ <IconButton
onClick={() =>
setSlCurrentlyEditing(original.uuid ?? null)
}
onKeyDown={handleKeyboardActivation(() =>
setSlCurrentlyEditing(original.uuid ?? null),
)}
- >
- <Icons.EditOutlined iconSize="l" />
- </span>
+ icon={<Icons.EditOutlined iconSize="l" />}
+ />
</Tooltip>
)}
- </Actions>
+ </div>
);
}
@@ -740,23 +725,21 @@ function DatabaseList({
return null;
}
return (
- <Actions className="actions">
+ <div className="actions">
{canEdit && (
<Tooltip
id="edit-action-tooltip"
title={t('Edit')}
placement="bottom"
>
- <span
- role="button"
+ <IconButton
data-test="database-edit"
- tabIndex={0}
- className="action-button"
onClick={handleEdit}
onKeyDown={handleKeyboardActivation(handleEdit)}
- >
- <Icons.EditOutlined data-test="edit-alt" iconSize="l" />
- </span>
+ icon={
+ <Icons.EditOutlined data-test="edit-alt" iconSize="l" />
+ }
+ />
</Tooltip>
)}
{canExport && (
@@ -765,15 +748,12 @@ function DatabaseList({
title={t('Export')}
placement="bottom"
>
- <span
- role="button"
- tabIndex={0}
- className="action-button"
+ <IconButton
+ data-test="database-export"
onClick={handleExport}
onKeyDown={handleKeyboardActivation(handleExport)}
- >
- <Icons.UploadOutlined iconSize="l" />
- </span>
+ icon={<Icons.UploadOutlined iconSize="l" />}
+ />
</Tooltip>
)}
{canEdit && (
@@ -782,37 +762,29 @@ function DatabaseList({
title={t('Sync Permissions')}
placement="bottom"
>
- <span
- role="button"
+ <IconButton
data-test="database-sync-perm"
- tabIndex={0}
- className="action-button"
onClick={handleSync}
onKeyDown={handleKeyboardActivation(handleSync)}
- >
- <Icons.SyncOutlined iconSize="l" />
- </span>
+ icon={<Icons.SyncOutlined iconSize="l" />}
+ />
</Tooltip>
)}
{canDelete && (
- <span
- role="button"
- tabIndex={0}
- className="action-button"
- data-test="database-delete"
- onClick={handleDelete}
- onKeyDown={handleKeyboardActivation(handleDelete)}
+ <Tooltip
+ id="delete-action-tooltip"
+ title={t('Delete %s', databaseLabelLower())}
+ placement="bottom"
>
- <Tooltip
- id="delete-action-tooltip"
- title={t('Delete %s', databaseLabelLower())}
- placement="bottom"
- >
- <Icons.DeleteOutlined iconSize="l" />
- </Tooltip>
- </span>
+ <IconButton
+ data-test="database-delete"
+ onClick={handleDelete}
+ onKeyDown={handleKeyboardActivation(handleDelete)}
+ icon={<Icons.DeleteOutlined iconSize="l" />}
+ />
+ </Tooltip>
)}
- </Actions>
+ </div>
);
},
Header: t('Actions'),
diff --git a/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx b/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx
index 169c869695d..354d16ab548 100644
--- a/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx
+++ b/superset-frontend/src/pages/DatasetList/DatasetList.behavior.test.tsx
@@ -21,6 +21,7 @@ import userEvent from '@testing-library/user-event';
import fetchMock from 'fetch-mock';
import rison from 'rison';
import { ComponentType } from 'react';
+import { mockUserSubjectsBootstrapData } from 'spec/helpers/mockBootstrapData';
import {
setupMocks,
renderDatasetList,
@@ -37,6 +38,10 @@ import {
jest.mock('src/utils/export');
+jest.mock('src/utils/getBootstrapData', () =>
+ mockUserSubjectsBootstrapData([1]),
+);
+
// Mock withToasts HOC to be a passthrough so we can spy on toast calls
jest.mock('src/components/MessageToasts/withToasts', () => ({
__esModule: true,
diff --git a/superset-frontend/src/pages/DatasetList/DatasetList.listview.test.tsx b/superset-frontend/src/pages/DatasetList/DatasetList.listview.test.tsx
index d3a7e5e1530..432cfe2a793 100644
--- a/superset-frontend/src/pages/DatasetList/DatasetList.listview.test.tsx
+++ b/superset-frontend/src/pages/DatasetList/DatasetList.listview.test.tsx
@@ -42,6 +42,7 @@ import {
mockDatasetListEndpoints,
mockDatasetEditors,
getDeleteRouteName,
+ mockEditorUser,
} from './DatasetList.testHelpers';
jest.mock('src/utils/getBootstrapData', () => ({
@@ -1021,11 +1022,10 @@ test('edit action is enabled for dataset editor', async () => {
const row = screen.getByText(dataset.table_name).closest('tr');
const editIcon = within(row!).getByTestId('edit');
- const editButton = editIcon.closest('.action-button, .disabled');
+ const editButton = editIcon.closest('[role="button"]');
- // Should have action-button class (not disabled)
- expect(editButton).toHaveClass('action-button');
- expect(editButton).not.toHaveClass('disabled');
+ // Should be enabled (not disabled)
+ expect(editButton).toHaveAttribute('aria-disabled', 'false');
});
test('edit action is disabled for non-editor', async () => {
@@ -1036,13 +1036,8 @@ test('edit action is disabled for non-editor', async () => {
mockDatasetListEndpoints({ result: [dataset], count: 1 });
- // Use a non-admin user to test the editor check.
- const regularUser = {
- ...mockAdminUser,
- roles: { Admin: [['can_read', 'Dataset']] },
- };
-
- renderDatasetList(regularUser);
+ // editors does not include mockEditorUser, so edit button should be disabled
+ renderDatasetList(mockEditorUser);
await waitFor(() => {
expect(screen.getByText(dataset.table_name)).toBeInTheDocument();
@@ -1050,17 +1045,15 @@ test('edit action is disabled for non-editor', async () => {
const row = screen.getByText(dataset.table_name).closest('tr');
const editIcon = within(row!).getByTestId('edit');
- const editButton = editIcon.closest('.action-button, .disabled');
+ const editButton = editIcon.closest('[role="button"]');
- // Should have disabled class (disabled buttons still have 'action-button' class)
- expect(editButton).toHaveClass('disabled');
- expect(editButton).toHaveClass('action-button');
+ // Should be disabled
+ expect(editButton).toHaveAttribute('aria-disabled', 'true');
});
test('all action buttons are clickable and enabled for admin user', async () => {
const virtualDataset = {
...mockDatasets[1],
- editors: [mockDatasetEditors.admin],
};
mockDatasetListEndpoints({ result: [virtualDataset], count: 1 });
@@ -1079,22 +1072,16 @@ test('all action buttons are clickable and enabled for admin user', async () =>
const editIcon = within(row!).getByTestId('edit');
const duplicateIcon = within(row!).getByTestId('copy');
- const deleteButton = deleteIcon.closest('.action-button, .disabled');
- const exportButton = exportIcon.closest('.action-button, .disabled');
- const editButton = editIcon.closest('.action-button, .disabled');
- const duplicateButton = duplicateIcon.closest('.action-button, .disabled');
-
- // All should have action-button class (enabled)
- expect(deleteButton).toHaveClass('action-button');
- expect(exportButton).toHaveClass('action-button');
- expect(editButton).toHaveClass('action-button');
- expect(duplicateButton).toHaveClass('action-button');
-
- // None should be disabled
- expect(deleteButton).not.toHaveClass('disabled');
- expect(exportButton).not.toHaveClass('disabled');
- expect(editButton).not.toHaveClass('disabled');
- expect(duplicateButton).not.toHaveClass('disabled');
+ const deleteButton = deleteIcon.closest('[role="button"]');
+ const exportButton = exportIcon.closest('[role="button"]');
+ const editButton = editIcon.closest('[role="button"]');
+ const duplicateButton = duplicateIcon.closest('[role="button"]');
+
+ // None should be disabled (export/duplicate never set aria-disabled at all)
+ expect(deleteButton).not.toHaveAttribute('aria-disabled', 'true');
+ expect(exportButton).not.toHaveAttribute('aria-disabled', 'true');
+ expect(editButton).not.toHaveAttribute('aria-disabled', 'true');
+ expect(duplicateButton).not.toHaveAttribute('aria-disabled', 'true');
});
test('displays error when initial dataset fetch fails with 500', async () => {
diff --git a/superset-frontend/src/pages/DatasetList/DatasetList.testHelpers.tsx b/superset-frontend/src/pages/DatasetList/DatasetList.testHelpers.tsx
index 6f897d3c4b9..63e87008376 100644
--- a/superset-frontend/src/pages/DatasetList/DatasetList.testHelpers.tsx
+++ b/superset-frontend/src/pages/DatasetList/DatasetList.testHelpers.tsx
@@ -225,6 +225,11 @@ export const mockAdminUser = {
userId: 1,
firstName: 'Admin',
lastName: 'User',
+ username: 'admin',
+ permissions: {
+ database_access: [],
+ datasource_access: [],
+ },
roles: {
Admin: [
['can_read', 'Dataset'],
diff --git a/superset-frontend/src/pages/DatasetList/index.tsx b/superset-frontend/src/pages/DatasetList/index.tsx
index 84c4d306d2d..21138750efc 100644
--- a/superset-frontend/src/pages/DatasetList/index.tsx
+++ b/superset-frontend/src/pages/DatasetList/index.tsx
@@ -78,7 +78,7 @@ import Subject from 'src/types/Subject';
import withToasts from 'src/components/MessageToasts/withToasts';
import { Icons } from '@superset-ui/core/components/Icons';
import WarningIconWithTooltip from '@superset-ui/core/components/WarningIconWithTooltip';
-import { isUserAdmin } from 'src/dashboard/util/permissionUtils';
+import { isUserEditorOrAdmin } from 'src/dashboard/util/permissionUtils';
import {
PAGE_SIZE,
@@ -100,9 +100,12 @@ import {
import { useSelector } from 'react-redux';
import { QueryObjectColumns } from 'src/views/CRUD/types';
import { WIDER_DROPDOWN_WIDTH } from 'src/components/ListView/utils';
-import type { BootstrapData } from 'src/types/bootstrapTypes';
+import type {
+ BootstrapData,
+ UserWithPermissionsAndRoles,
+} from 'src/types/bootstrapTypes';
import type User from 'src/types/User';
-import getBootstrapData from 'src/utils/getBootstrapData';
+import IconButton from 'src/dashboard/components/IconButton';
const SEMANTIC_LAYERS_FLAG = 'SEMANTIC_LAYERS' as FeatureFlag;
type DatasetExtra = {
@@ -133,27 +136,6 @@ const FlexRowContainer = styled.div`
const Actions = styled.div`
${({ theme }) => css`
color: ${theme.colorIcon};
-
- .disabled {
- svg,
- i {
- &:hover {
- path {
- fill: ${theme.colorText};
- }
- }
- }
- color: ${theme.colorTextDisabled};
- &:hover {
- cursor: not-allowed;
- }
- .ant-menu-item:hover {
- cursor: default;
- }
- &::after {
- color: ${theme.colorTextDisabled};
- }
- }
`}
`;
@@ -187,11 +169,7 @@ interface VirtualDataset extends Dataset {
interface DatasetListProps {
addDangerToast: (msg: string) => void;
addSuccessToast: (msg: string) => void;
- user: {
- userId: string | number;
- firstName: string;
- lastName: string;
- };
+ user?: UserWithPermissionsAndRoles;
}
type RelatedObjects = {
@@ -226,10 +204,6 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
// can inspect them when a different filter changes.
const currentTypeFilter = useRef<unknown>(undefined);
const currentConnectionFilter = useRef<unknown>(undefined);
- const userSubjects = useMemo(
- () => new Set(getBootstrapData()?.common?.user_subjects ?? []),
- [],
- );
// Ref wired to ListView's filter controls for programmatic per-filter clearing.
const filtersRef = useRef<{
@@ -853,6 +827,8 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
Cell: ({ row: { original } }: CellProps<Dataset>) => {
const isSemanticView = original.kind === 'semantic_view';
+ const allowEdit = isUserEditorOrAdmin(user, original.editors);
+
// Semantic view: show edit and delete buttons
if (isSemanticView) {
if (!canEdit && !canDelete) return null;
@@ -861,41 +837,47 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
{canDelete && (
<Tooltip
id="delete-action-tooltip"
- title={t('Delete')}
+ title={
+ allowEdit
+ ? t('Delete')
+ : t(
+ 'You must be a dataset editor in order to delete. Please reach out to a dataset editor to request modifications or edit access.',
+ )
+ }
placement="bottom"
>
- <span
+ <IconButton
data-test="dataset-row-delete"
- role="button"
- tabIndex={0}
- className="action-button"
+ disabled={!allowEdit}
onClick={() => handleSemanticViewDelete(original)}
onKeyDown={handleKeyboardActivation(() =>
handleSemanticViewDelete(original),
)}
- >
- <Icons.DeleteOutlined iconSize="l" />
- </span>
+ icon={<Icons.DeleteOutlined iconSize="l" />}
+ />
</Tooltip>
)}
{canEdit && (
<Tooltip
id="edit-action-tooltip"
- title={t('Edit')}
+ title={
+ allowEdit
+ ? t('Edit')
+ : t(
+ 'You must be a dataset editor in order to edit. Please reach out to a dataset editor to request modifications or edit access.',
+ )
+ }
placement="bottom"
>
- <span
+ <IconButton
data-test="dataset-row-edit"
- role="button"
- tabIndex={0}
- className="action-button"
+ disabled={!allowEdit}
onClick={() => setSvCurrentlyEditing(original)}
onKeyDown={handleKeyboardActivation(() =>
setSvCurrentlyEditing(original),
)}
- >
- <Icons.EditOutlined iconSize="l" />
- </span>
+ icon={<Icons.EditOutlined iconSize="l" />}
+ />
</Tooltip>
)}
</Actions>
@@ -903,11 +885,6 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
}
// Dataset: full set of actions
- const allowEdit =
- original.editors
- ?.map((o: Subject) => o.id)
- .some((id: number) => userSubjects.has(id)) || isUserAdmin(user);
-
const handleEdit = () => openDatasetEditModal(original);
const handleDelete = () => openDatasetDeleteModal(original);
const handleExport = () => handleBulkDatasetExport([original]);
@@ -933,20 +910,17 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
}
placement="bottom"
>
- <span
+ <IconButton
data-test="dataset-row-edit"
- role="button"
- tabIndex={0}
- className={`action-button ${allowEdit ? '' : 'disabled'}`}
- onClick={allowEdit ? handleEdit : undefined}
+ disabled={!allowEdit}
+ onClick={handleEdit}
onKeyDown={
allowEdit
? handleKeyboardActivation(handleEdit)
: undefined
}
- >
- <Icons.EditOutlined iconSize="l" />
- </span>
+ icon={<Icons.EditOutlined iconSize="l" />}
+ />
</Tooltip>
)}
{canExport && (
@@ -955,16 +929,12 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
title={t('Export')}
placement="bottom"
>
- <span
+ <IconButton
data-test="dataset-row-export"
- role="button"
- tabIndex={0}
- className="action-button"
onClick={handleExport}
onKeyDown={handleKeyboardActivation(handleExport)}
- >
- <Icons.UploadOutlined iconSize="l" />
- </span>
+ icon={<Icons.UploadOutlined iconSize="l" />}
+ />
</Tooltip>
)}
{canDuplicate && original.kind === 'virtual' && (
@@ -973,34 +943,33 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
title={t('Duplicate')}
placement="bottom"
>
- <span
+ <IconButton
data-test="dataset-row-duplicate"
- role="button"
- tabIndex={0}
- className="action-button"
onClick={handleDuplicate}
onKeyDown={handleKeyboardActivation(handleDuplicate)}
- >
- <Icons.CopyOutlined iconSize="l" />
- </span>
+ icon={<Icons.CopyOutlined iconSize="l" />}
+ />
</Tooltip>
)}
{canDelete && (
<Tooltip
id="delete-action-tooltip"
- title={t('Delete')}
+ title={
+ allowEdit
+ ? t('Delete')
+ : t(
+ 'You must be a dataset editor in order to delete. Please reach out to a dataset editor to request modifications or edit access.',
+ )
+ }
placement="bottom"
>
- <span
+ <IconButton
data-test="dataset-row-delete"
- role="button"
- tabIndex={0}
- className="action-button"
+ disabled={!allowEdit}
onClick={handleDelete}
onKeyDown={handleKeyboardActivation(handleDelete)}
- >
- <Icons.DeleteOutlined iconSize="l" />
- </span>
+ icon={<Icons.DeleteOutlined iconSize="l" />}
+ />
</Tooltip>
)}
</Actions>
@@ -1028,7 +997,6 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
handleBulkDatasetExport,
PREVENT_UNSAFE_DEFAULT_URLS_ON_DATASET,
user,
- userSubjects,
],
);
diff --git a/superset-frontend/src/pages/Home/index.tsx b/superset-frontend/src/pages/Home/index.tsx
index fb7e646d50e..f54245958f2 100644
--- a/superset-frontend/src/pages/Home/index.tsx
+++ b/superset-frontend/src/pages/Home/index.tsx
@@ -27,7 +27,7 @@ import {
import { styled } from '@apache-superset/core/theme';
import rison from 'rison';
import { Collapse, ListViewCard } from '@superset-ui/core/components';
-import { User } from 'src/types/bootstrapTypes';
+import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
import { reject } from 'lodash-es';
import {
dangerouslyGetItemDoNotUse,
@@ -59,7 +59,7 @@ import DashboardTable from 'src/features/home/DashboardTable';
const extensionsRegistry = getExtensionsRegistry();
interface WelcomeProps {
- user: User;
+ user: UserWithPermissionsAndRoles;
addDangerToast: (arg0: string) => void;
}
diff --git a/superset-frontend/src/views/CRUD/types.ts b/superset-frontend/src/views/CRUD/types.ts
index 30f191f3b61..b2900e5153c 100644
--- a/superset-frontend/src/views/CRUD/types.ts
+++ b/superset-frontend/src/views/CRUD/types.ts
@@ -17,7 +17,7 @@
* under the License.
*/
import { QueryState } from '@superset-ui/core';
-import { User as BootstrapUser } from 'src/types/bootstrapTypes';
+import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
import Database from 'src/types/Database';
import User from 'src/types/User';
import Subject from 'src/types/Subject';
@@ -44,7 +44,7 @@ export type Filter = {
export interface DashboardTableProps {
addDangerToast: (message: string) => void;
addSuccessToast: (message: string) => void;
- user?: BootstrapUser;
+ user?: UserWithPermissionsAndRoles;
mine: Array<Dashboard>;
showThumbnails?: boolean;
otherTabData?: Array<Dashboard>;
diff --git a/superset-frontend/src/views/CRUD/utils.tsx b/superset-frontend/src/views/CRUD/utils.tsx
index 649d33391f5..baff54d29a9 100644
--- a/superset-frontend/src/views/CRUD/utils.tsx
+++ b/superset-frontend/src/views/CRUD/utils.tsx
@@ -86,7 +86,7 @@ const createFetchResourceMethod =
resource: string,
relation: string,
handleError: (error: Response) => void,
- user?: { userId: string | number; firstName: string; lastName: string },
+ user?: { userId?: string | number; firstName: string; lastName: string },
) =>
async (filterValue = '', page: number, pageSize: number) => {
const resourceEndpoint = `/api/v1/${resource}/${method}/${relation}`;
@@ -101,12 +101,13 @@ const createFetchResourceMethod =
let fetchedLoggedUser = false;
let loggedUserExtra: Record<string, unknown> | undefined;
- const loggedUser = user
- ? {
- label: `${user.firstName} ${user.lastName}`,
- value: user.userId,
- }
- : undefined;
+ const loggedUser =
+ user?.userId === undefined
+ ? undefined
+ : {
+ label: `${user.firstName} ${user.lastName}`,
+ value: user.userId,
+ };
const data: {
label: string;
@@ -271,7 +272,7 @@ const createFetchSubjectRelation =
(
resource: string,
handleError: (error: Response) => void,
- user?: { userId: string | number; firstName: string; lastName: string },
+ user?: { userId?: string | number; firstName: string; lastName: string },
) => {
const currentUserSubjectId = getBootstrapData()?.common?.user_subject_id;
const subjectUser =