Compare commits

...

3 Commits

Author SHA1 Message Date
sadpandajoe
e7352a603c chore: add screenshot for PR body (will be removed before merge) 2026-04-10 05:54:58 +00:00
sadpandajoe
07629154db fix(dashboard): address review issues in ShareDashboardModal
MAJOR-1: Validate email input contains '@' before adding; show inline error
MAJOR-2: Show 'Done' label when no invite emails queued (clarifies intent)
MAJOR-3: Simplify Share button disabled to `isSubmitting || !dashboardUrl`
MAJOR-4: Add keyDownAddedRef guard to prevent onBlur double-add after Enter
MAJOR-5: Use permalinkFetchedRef so URL is generated once per modal open
MAJOR-6: Handle 404 from invite endpoint gracefully with specific error message
MINOR-1: Merge duplicate react-redux imports into single statement
MINOR-2: Complete team-admin invite test — clicks Share, asserts API call,
         success toast, and onHide
MINOR-3: Add workspace-admin happy-path end-to-end invite test

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 04:59:40 +00:00
sadpandajoe
ed2a2f2afa feat(dashboard): add ShareDashboardModal with invite flow and correct button capitalization
- Add ShareDashboardModal component with 'Copy link', 'Cancel', 'Share' button text
- Add isUserTeamAdmin() utility so team admins (not just workspace admins) can access the invite flow
- Wire modal into dashboard Header with showingShareModal state and show/hide callbacks
- Add ShareDashboardModal menu item ('Share dashboard...') to header actions dropdown
- Add ShareDashboardModal key to MenuKeys enum
- Add tests covering button capitalization, admin/team-admin invite visibility, and clipboard copy

Closes sc-83062

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 04:06:26 +00:00
8 changed files with 618 additions and 1 deletions

BIN
sc-83062-after.png Normal file

Binary file not shown.

After

Width:  |  Height:  |  Size: 28 KiB

View File

@@ -62,6 +62,7 @@ import {
import { PageHeaderWithActions } from '@superset-ui/core/components/PageHeaderWithActions';
import { useUnsavedChangesPrompt } from 'src/hooks/useUnsavedChangesPrompt';
import DashboardEmbedModal from '../EmbeddedModal';
import ShareDashboardModal from '../ShareDashboardModal';
import OverwriteConfirm from '../OverwriteConfirm';
import {
addDangerToast,
@@ -230,6 +231,7 @@ const Header = (): JSX.Element => {
const [showingRefreshModal, setShowingRefreshModal] = useState(false);
const [showingEmbedModal, setShowingEmbedModal] = useState(false);
const [showingReportModal, setShowingReportModal] = useState(false);
const [showingShareModal, setShowingShareModal] = useState(false);
const [currentReportDeleting, setCurrentReportDeleting] =
useState<AlertObject | null>(null);
const dashboardInfo = useSelector(
@@ -531,6 +533,14 @@ const Header = (): JSX.Element => {
setShowingEmbedModal(false);
}, []);
const showShareModal = useCallback(() => {
setShowingShareModal(true);
}, []);
const hideShareModal = useCallback(() => {
setShowingShareModal(false);
}, []);
const showReportModal = useCallback(() => {
setShowingReportModal(true);
}, []);
@@ -815,6 +825,7 @@ const Header = (): JSX.Element => {
showReportModal,
showPropertiesModal,
showRefreshModal,
showShareModal,
setCurrentReportDeleting,
manageEmbedded: showEmbedModal,
lastModifiedTime: actualLastModifiedTime,
@@ -901,6 +912,18 @@ const Header = (): JSX.Element => {
dashboardId={String(dashboardInfo.id)}
/>
)}
{userCanShare && (
<ShareDashboardModal
show={showingShareModal}
onHide={hideShareModal}
dashboardId={dashboardInfo.id}
dashboardTitle={dashboardTitle ?? ''}
addSuccessToast={boundActionCreators.addSuccessToast}
addDangerToast={boundActionCreators.addDangerToast}
user={user}
/>
)}
<Global
styles={css`
.ant-menu-vertical {

View File

@@ -49,6 +49,7 @@ export interface HeaderDropdownProps {
shouldPersistRefreshFrequency: boolean;
showPropertiesModal: () => void;
showRefreshModal: () => void;
showShareModal: () => void;
userCanEdit: boolean | undefined;
userCanSave: boolean | undefined;
userCanShare: boolean | undefined;

View File

@@ -62,6 +62,7 @@ export const useHeaderActionsMenu = ({
showPropertiesModal,
showRefreshModal,
showReportModal,
showShareModal,
manageEmbedded,
dashboardTitle,
logEvent,
@@ -111,6 +112,9 @@ export const useHeaderActionsMenu = ({
case MenuKeys.ManageEmbedded:
manageEmbedded();
break;
case MenuKeys.ShareDashboardModal:
showShareModal();
break;
default:
break;
}
@@ -121,6 +125,7 @@ export const useHeaderActionsMenu = ({
addSuccessToast,
showPropertiesModal,
showRefreshModal,
showShareModal,
manageEmbedded,
history,
],
@@ -266,11 +271,20 @@ export const useHeaderActionsMenu = ({
// Download submenu
menuItems.push(downloadMenuItem);
// Share submenu
// Share submenu (permalink copy/email)
if (userCanShare) {
menuItems.push(shareMenuItems);
}
// Share dashboard modal (invite flow)
if (userCanShare) {
menuItems.push({
key: MenuKeys.ShareDashboardModal,
label: t('Share dashboard...'),
disabled: isLoading,
});
}
// Embed dashboard
if (!editMode && userCanCurate) {
menuItems.push({

View File

@@ -0,0 +1,220 @@
/**
* 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,
userEvent,
waitFor,
} from 'spec/helpers/testing-library';
import { SupersetClient } from '@superset-ui/core';
import * as copyTextToClipboard from 'src/utils/copy';
import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
import ShareDashboardModal from '.';
jest.mock('src/utils/copy', () => ({
__esModule: true,
default: jest.fn().mockResolvedValue(undefined),
}));
jest.mock('src/utils/urlUtils', () => ({
...jest.requireActual<any>('src/utils/urlUtils'),
getDashboardPermalink: jest
.fn()
.mockResolvedValue({ url: 'http://localhost/superset/dashboard/p/abc/' }),
}));
jest.mock('@superset-ui/core', () => ({
...jest.requireActual<any>('@superset-ui/core'),
SupersetClient: {
post: jest.fn().mockResolvedValue({}),
},
}));
const mockOnHide = jest.fn();
const mockAddSuccessToast = jest.fn();
const mockAddDangerToast = jest.fn();
const baseProps = {
dashboardId: 1,
dashboardTitle: 'Test Dashboard',
show: true,
onHide: mockOnHide,
addSuccessToast: mockAddSuccessToast,
addDangerToast: mockAddDangerToast,
};
const makeUser = (roleNames: string[]): UserWithPermissionsAndRoles => ({
userId: 1,
username: 'testuser',
firstName: 'Test',
lastName: 'User',
isActive: true,
email: 'test@example.com',
roles: Object.fromEntries(roleNames.map(name => [name, []])),
permissions: {},
});
const reduxState = {
dataMask: {},
dashboardState: {
activeTabs: [],
chartStates: {},
},
sliceEntities: { slices: {} },
};
const setup = (extraProps = {}) =>
render(<ShareDashboardModal {...baseProps} {...extraProps} />, {
useRedux: true,
initialState: reduxState,
});
beforeEach(() => {
jest.clearAllMocks();
});
test('renders Cancel and Share buttons with correct capitalization', async () => {
setup();
expect(
await screen.findByRole('button', { name: 'Cancel' }),
).toBeInTheDocument();
expect(screen.getByRole('button', { name: 'Done' })).toBeInTheDocument();
});
test('renders Copy link button with correct capitalization', async () => {
setup();
expect(
await screen.findByRole('button', { name: 'Copy link' }),
).toBeInTheDocument();
});
test('Share button label is Share (not Done) when invite emails are queued', async () => {
setup({ user: makeUser(['Admin']) });
const emailInput = await screen.findByTestId('share-dashboard-email-input');
await userEvent.type(emailInput, 'user@example.com{enter}');
expect(await screen.findByRole('button', { name: 'Share' })).toBeInTheDocument();
});
test('does not render invite section for regular users', async () => {
setup({ user: makeUser(['Alpha']) });
await screen.findByRole('button', { name: 'Copy link' });
expect(
screen.queryByTestId('share-dashboard-email-input'),
).not.toBeInTheDocument();
expect(screen.queryByText('Invite people')).not.toBeInTheDocument();
});
test('renders invite section for workspace admins (Admin role)', async () => {
setup({ user: makeUser(['Admin']) });
expect(
await screen.findByTestId('share-dashboard-email-input'),
).toBeInTheDocument();
expect(screen.getByText('Invite people')).toBeInTheDocument();
});
test('renders invite section for team admins (Team Admin role)', async () => {
setup({ user: makeUser(['Team Admin']) });
expect(
await screen.findByTestId('share-dashboard-email-input'),
).toBeInTheDocument();
expect(screen.getByText('Invite people')).toBeInTheDocument();
});
test('Copy link button copies the dashboard URL to clipboard', async () => {
setup();
const copyBtn = await screen.findByRole('button', { name: 'Copy link' });
await userEvent.click(copyBtn);
await waitFor(() => {
expect(copyTextToClipboard.default).toHaveBeenCalledTimes(1);
});
});
test('Cancel button calls onHide', async () => {
setup();
const cancelBtn = await screen.findByRole('button', { name: 'Cancel' });
await userEvent.click(cancelBtn);
expect(mockOnHide).toHaveBeenCalledTimes(1);
});
test('Done button (no emails) calls onHide without posting invite', async () => {
setup({ user: makeUser(['Alpha']) });
const doneBtn = await screen.findByRole('button', { name: 'Done' });
await userEvent.click(doneBtn);
expect(mockOnHide).toHaveBeenCalledTimes(1);
expect(SupersetClient.post).not.toHaveBeenCalled();
});
test('rejects email input without @ and shows validation error', async () => {
setup({ user: makeUser(['Admin']) });
const emailInput = await screen.findByTestId('share-dashboard-email-input');
await userEvent.type(emailInput, 'notanemail{enter}');
expect(
await screen.findByTestId('share-dashboard-email-error'),
).toBeInTheDocument();
expect(screen.queryByText('notanemail')).not.toBeInTheDocument();
});
// MINOR-2 fix: complete the team admin invite flow end-to-end
test('team admin can add email and trigger invite on Share', async () => {
setup({ user: makeUser(['Team Admin']) });
const emailInput = await screen.findByTestId('share-dashboard-email-input');
await userEvent.type(emailInput, 'invitee@example.com{enter}');
expect(await screen.findByText('invitee@example.com')).toBeInTheDocument();
const shareBtn = await screen.findByRole('button', { name: 'Share' });
await userEvent.click(shareBtn);
await waitFor(() => {
expect(SupersetClient.post).toHaveBeenCalledWith(
expect.objectContaining({
endpoint: '/api/v1/security/users/invite',
jsonPayload: expect.objectContaining({
emails: ['invitee@example.com'],
}),
}),
);
});
expect(mockAddSuccessToast).toHaveBeenCalled();
expect(mockOnHide).toHaveBeenCalled();
});
// MINOR-3: happy path end-to-end for workspace admin
test('workspace admin sends invite successfully', async () => {
setup({ user: makeUser(['Admin']) });
const emailInput = await screen.findByTestId('share-dashboard-email-input');
await userEvent.type(emailInput, 'new.user@example.com{enter}');
expect(await screen.findByText('new.user@example.com')).toBeInTheDocument();
await userEvent.click(screen.getByRole('button', { name: 'Share' }));
await waitFor(() => {
expect(SupersetClient.post).toHaveBeenCalledWith(
expect.objectContaining({
endpoint: '/api/v1/security/users/invite',
jsonPayload: expect.objectContaining({
emails: ['new.user@example.com'],
dashboard_id: 1,
dashboard_title: 'Test Dashboard',
}),
}),
);
expect(mockAddSuccessToast).toHaveBeenCalled();
expect(mockOnHide).toHaveBeenCalled();
});
});

View File

@@ -0,0 +1,349 @@
/**
* 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 { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { t } from '@apache-superset/core/translation';
import { styled } from '@apache-superset/core/theme';
import { SupersetClient, logging } from '@superset-ui/core';
import { Button, Input, Modal, Space, Tag } from '@superset-ui/core/components';
import { getDashboardPermalink } from 'src/utils/urlUtils';
import copyTextToClipboard from 'src/utils/copy';
import {
isUserAdmin,
isUserTeamAdmin,
} from 'src/dashboard/util/permissionUtils';
import {
UserWithPermissionsAndRoles,
UndefinedUser,
} from 'src/types/bootstrapTypes';
import { useSelector, shallowEqual } from 'react-redux';
import { RootState } from 'src/dashboard/types';
import { hasStatefulCharts } from 'src/dashboard/util/chartStateConverter';
export type ShareDashboardModalProps = {
dashboardId: number;
dashboardTitle: string;
show: boolean;
onHide: () => void;
addSuccessToast: (message: string) => void;
addDangerToast: (message: string) => void;
user?: UserWithPermissionsAndRoles | UndefinedUser;
};
const SectionTitle = styled.p`
font-weight: ${({ theme }) => theme.typography.weights.bold};
margin-bottom: ${({ theme }) => theme.gridUnit * 2}px;
`;
const LinkRow = styled.div`
display: flex;
align-items: center;
gap: ${({ theme }) => theme.gridUnit * 2}px;
margin-bottom: ${({ theme }) => theme.gridUnit * 4}px;
`;
const LinkInput = styled(Input)`
flex: 1;
background-color: ${({ theme }) => theme.colors.grayscale.light4};
`;
const EmailInputRow = styled.div`
display: flex;
align-items: flex-start;
gap: ${({ theme }) => theme.gridUnit * 2}px;
margin-bottom: ${({ theme }) => theme.gridUnit * 2}px;
`;
const EmailTagsContainer = styled.div`
display: flex;
flex-wrap: wrap;
gap: ${({ theme }) => theme.gridUnit}px;
margin-bottom: ${({ theme }) => theme.gridUnit * 2}px;
min-height: ${({ theme }) => theme.gridUnit * 4}px;
`;
const HintText = styled.p`
color: ${({ theme }) => theme.colors.grayscale.base};
font-size: ${({ theme }) => theme.typography.sizes.s}px;
margin-bottom: 0;
`;
const ErrorText = styled.p`
color: ${({ theme }) => theme.colors.error.base};
font-size: ${({ theme }) => theme.typography.sizes.s}px;
margin-top: ${({ theme }) => theme.gridUnit}px;
margin-bottom: 0;
`;
const ShareDashboardModal = ({
dashboardId,
dashboardTitle,
show,
onHide,
addSuccessToast,
addDangerToast,
user,
}: ShareDashboardModalProps) => {
const [dashboardUrl, setDashboardUrl] = useState('');
const [emailInput, setEmailInput] = useState('');
const [emailError, setEmailError] = useState('');
const [inviteEmails, setInviteEmails] = useState<string[]>([]);
const [isSubmitting, setIsSubmitting] = useState(false);
// MAJOR-4: guard against onBlur double-add after Enter/comma keydown
const keyDownAddedRef = useRef(false);
// MAJOR-5: only fetch the permalink once per modal open
const permalinkFetchedRef = useRef(false);
const { dataMask, activeTabs, chartStates, sliceEntities } = useSelector(
(state: RootState) => ({
dataMask: state.dataMask,
activeTabs: state.dashboardState.activeTabs,
chartStates: state.dashboardState.chartStates,
sliceEntities: state.sliceEntities?.slices,
}),
shallowEqual,
);
const canInviteUsers = useMemo(
() => isUserAdmin(user) || isUserTeamAdmin(user),
[user],
);
// MAJOR-5: generate permalink only once when the modal transitions to open
useEffect(() => {
if (!show) {
permalinkFetchedRef.current = false;
return;
}
if (permalinkFetchedRef.current) return;
permalinkFetchedRef.current = true;
const includeChartState =
hasStatefulCharts(sliceEntities) &&
chartStates &&
Object.keys(chartStates).length > 0;
getDashboardPermalink({
dashboardId,
dataMask,
activeTabs,
chartStates: includeChartState ? chartStates : undefined,
includeChartState,
})
.then(result => {
if (result?.url) setDashboardUrl(result.url);
})
.catch(err => {
logging.error(err);
});
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [show]);
const handleCopyLink = useCallback(async () => {
try {
await copyTextToClipboard(() => Promise.resolve(dashboardUrl));
addSuccessToast(t('Copied to clipboard!'));
} catch (error) {
logging.error(error);
addDangerToast(t('Sorry, something went wrong. Try again later.'));
}
}, [dashboardUrl, addSuccessToast, addDangerToast]);
// MAJOR-1: validate that input contains '@' before adding to the list
const handleAddEmail = useCallback(() => {
const trimmed = emailInput.trim();
if (!trimmed) return;
if (!trimmed.includes('@')) {
setEmailError(t('Please enter a valid email address'));
return;
}
setEmailError('');
if (!inviteEmails.includes(trimmed)) {
setInviteEmails(prev => [...prev, trimmed]);
}
setEmailInput('');
}, [emailInput, inviteEmails]);
const handleEmailKeyDown = useCallback(
(e: React.KeyboardEvent<HTMLInputElement>) => {
if (e.key === 'Enter' || e.key === ',') {
e.preventDefault();
// MAJOR-4: flag that we handled this via keydown so onBlur is a no-op
keyDownAddedRef.current = true;
handleAddEmail();
}
},
[handleAddEmail],
);
// MAJOR-4: skip the blur handler if keydown already committed the value
const handleEmailBlur = useCallback(() => {
if (keyDownAddedRef.current) {
keyDownAddedRef.current = false;
return;
}
handleAddEmail();
}, [handleAddEmail]);
const handleRemoveEmail = useCallback((email: string) => {
setInviteEmails(prev => prev.filter(e => e !== email));
}, []);
const handleShare = useCallback(async () => {
if (!canInviteUsers || inviteEmails.length === 0) {
onHide();
return;
}
setIsSubmitting(true);
try {
await SupersetClient.post({
endpoint: '/api/v1/security/users/invite',
jsonPayload: {
emails: inviteEmails,
dashboard_id: dashboardId,
dashboard_url: dashboardUrl,
dashboard_title: dashboardTitle,
},
});
addSuccessToast(t('Invitation sent to %s', inviteEmails.join(', ')));
onHide();
} catch (error: any) {
logging.error(error);
// MAJOR-6: the invite endpoint may not exist in all deployments; degrade gracefully
if (error?.status === 404) {
addDangerToast(
t('User invitation is not supported in this deployment.'),
);
} else {
addDangerToast(t('Failed to send invitations. Please try again.'));
}
} finally {
setIsSubmitting(false);
}
}, [
canInviteUsers,
inviteEmails,
dashboardId,
dashboardUrl,
dashboardTitle,
addSuccessToast,
addDangerToast,
onHide,
]);
const handleHide = useCallback(() => {
setEmailInput('');
setEmailError('');
setInviteEmails([]);
onHide();
}, [onHide]);
// MAJOR-2: show 'Done' when there are no emails to send so intent is clear
const shareButtonLabel =
canInviteUsers && inviteEmails.length === 0 ? t('Done') : t('Share');
const footer = (
<Space>
<Button
key="cancel"
buttonStyle="secondary"
onClick={handleHide}
data-test="share-dashboard-modal-cancel"
>
{t('Cancel')}
</Button>
{/* MAJOR-3: disabled only when submitting or URL not yet loaded */}
<Button
key="share"
buttonStyle="primary"
onClick={handleShare}
loading={isSubmitting}
disabled={isSubmitting || !dashboardUrl}
data-test="share-dashboard-modal-share"
>
{shareButtonLabel}
</Button>
</Space>
);
return (
<Modal
show={show}
onHide={handleHide}
title={t('Share dashboard')}
footer={footer}
data-test="share-dashboard-modal"
>
<SectionTitle>{t('Dashboard link')}</SectionTitle>
<LinkRow>
<LinkInput
value={dashboardUrl}
readOnly
data-test="share-dashboard-url-input"
/>
<Button
buttonStyle="secondary"
onClick={handleCopyLink}
disabled={!dashboardUrl}
data-test="share-dashboard-copy-link"
>
{t('Copy link')}
</Button>
</LinkRow>
{canInviteUsers && (
<>
<SectionTitle>{t('Invite people')}</SectionTitle>
<EmailTagsContainer data-test="share-dashboard-email-tags">
{inviteEmails.map(email => (
<Tag
key={email}
closable
onClose={() => handleRemoveEmail(email)}
>
{email}
</Tag>
))}
</EmailTagsContainer>
<EmailInputRow>
<Input
value={emailInput}
onChange={e => setEmailInput(e.target.value)}
onKeyDown={handleEmailKeyDown}
onBlur={handleEmailBlur}
placeholder={t('Enter email address')}
data-test="share-dashboard-email-input"
/>
</EmailInputRow>
{emailError && (
<ErrorText data-test="share-dashboard-email-error">
{emailError}
</ErrorText>
)}
<HintText>
{t('Press Enter or comma to add multiple email addresses.')}
</HintText>
</>
)}
</Modal>
);
};
export default ShareDashboardModal;

View File

@@ -393,4 +393,5 @@ export enum MenuKeys {
ManageEmailReports = 'manage_email_reports',
ExportPivotXlsx = 'export_pivot_xlsx',
EmbedCode = 'embed_code',
ShareDashboardModal = 'share_dashboard_modal',
}

View File

@@ -28,6 +28,7 @@ import { findPermission } from 'src/utils/findPermission';
// this should really be a config value,
// but is hardcoded in backend logic already, so...
const ADMIN_ROLE_NAME = 'admin';
const TEAM_ADMIN_ROLE_NAME = 'team admin';
export const isUserAdmin = (
user?: UserWithPermissionsAndRoles | UndefinedUser,
@@ -37,6 +38,14 @@ export const isUserAdmin = (
role => role.toLowerCase() === ADMIN_ROLE_NAME,
);
export const isUserTeamAdmin = (
user?: UserWithPermissionsAndRoles | UndefinedUser,
) =>
isUserWithPermissionsAndRoles(user) &&
Object.keys(user.roles || {}).some(
role => role.toLowerCase() === TEAM_ADMIN_ROLE_NAME,
);
const isUserDashboardOwner = (
dashboard: Dashboard,
user: UserWithPermissionsAndRoles | UndefinedUser,