Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.x] [Workspace]Restrict at least one data source in workspace creation page #8468

Merged
merged 1 commit into from
Oct 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/8461.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feat:
- [Workspace]Restrict at least one data source in workspace creation page ([#8461](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8461))
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
MAX_WORKSPACE_DESCRIPTION_LENGTH,
MAX_WORKSPACE_NAME_LENGTH,
} from '../../../common/constants';
import { DataSourceConnectionType } from '../../../common/types';
import { WorkspaceCreateActionPanel } from './workspace_create_action_panel';

const mockApplication = applicationServiceMock.createStartContract();
Expand All @@ -18,16 +19,28 @@ describe('WorkspaceCreateActionPanel', () => {
const formData = {
name: 'Test Workspace',
description: 'This is a test workspace',
selectedDataSourceConnections: [
{
id: 'data-source-1',
name: 'Data Source 1',
type: '',
connectionType: DataSourceConnectionType.OpenSearchConnection,
},
],
};

it('should disable the "Create Workspace" button when name exceeds the maximum length', () => {
const longName = 'a'.repeat(MAX_WORKSPACE_NAME_LENGTH + 1);
render(
<WorkspaceCreateActionPanel
formId={formId}
formData={{ name: longName, description: formData.description }}
formData={{
...formData,
name: longName,
}}
application={mockApplication}
isSubmitting={false}
dataSourceEnabled
/>
);
const createButton = screen.getByText('Create workspace');
Expand All @@ -39,22 +52,61 @@ describe('WorkspaceCreateActionPanel', () => {
render(
<WorkspaceCreateActionPanel
formId={formId}
formData={{ name: formData.name, description: longDescription }}
formData={{
...formData,
description: longDescription,
}}
application={mockApplication}
isSubmitting={false}
dataSourceEnabled
/>
);
const createButton = screen.getByText('Create workspace');
expect(createButton.closest('button')).toBeDisabled();
});

it('should disable the "Create Workspace" button when data source enabled and no data sources selected', () => {
render(
<WorkspaceCreateActionPanel
formId={formId}
formData={{
...formData,
selectedDataSourceConnections: [],
}}
application={mockApplication}
isSubmitting={false}
dataSourceEnabled
/>
);
const createButton = screen.getByText('Create workspace');
expect(createButton.closest('button')).toBeDisabled();
});

it('should enable the "Create Workspace" button when no data sources selected but no data source enabled', () => {
render(
<WorkspaceCreateActionPanel
formId={formId}
formData={{
...formData,
selectedDataSourceConnections: [],
}}
application={mockApplication}
isSubmitting={false}
dataSourceEnabled={false}
/>
);
const createButton = screen.getByText('Create workspace');
expect(createButton.closest('button')).not.toBeDisabled();
});

it('should enable the "Create Workspace" button when name and description are within the maximum length', () => {
render(
<WorkspaceCreateActionPanel
formId={formId}
formData={formData}
application={mockApplication}
isSubmitting={false}
dataSourceEnabled
/>
);
const createButton = screen.getByText('Create workspace');
Expand All @@ -65,9 +117,10 @@ describe('WorkspaceCreateActionPanel', () => {
render(
<WorkspaceCreateActionPanel
formId={formId}
formData={{ name: 'test' }}
formData={formData}
application={mockApplication}
isSubmitting
dataSourceEnabled
/>
);
expect(screen.getByText('Create workspace').closest('button')).toBeDisabled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,26 @@ import {

interface WorkspaceCreateActionPanelProps {
formId: string;
formData: Pick<WorkspaceFormDataState, 'name' | 'description'>;
formData: Pick<WorkspaceFormDataState, 'name' | 'description' | 'selectedDataSourceConnections'>;
application: ApplicationStart;
isSubmitting: boolean;
dataSourceEnabled: boolean;
}

export const WorkspaceCreateActionPanel = ({
formId,
formData,
application,
isSubmitting,
dataSourceEnabled,
}: WorkspaceCreateActionPanelProps) => {
const [isCancelModalVisible, setIsCancelModalVisible] = useState(false);
const closeCancelModal = useCallback(() => setIsCancelModalVisible(false), []);
const showCancelModal = useCallback(() => setIsCancelModalVisible(true), []);
const createButtonDisabled =
(formData.name?.length ?? 0) > MAX_WORKSPACE_NAME_LENGTH ||
(formData.description?.length ?? 0) > MAX_WORKSPACE_DESCRIPTION_LENGTH;
(formData.description?.length ?? 0) > MAX_WORKSPACE_DESCRIPTION_LENGTH ||
(dataSourceEnabled && formData.selectedDataSourceConnections.length === 0);

return (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,9 @@ jest.spyOn(utils, 'fetchDataSourceConnections').mockImplementation(async (passed

const WorkspaceCreator = ({
isDashboardAdmin = false,
dataSourceEnabled = false,
...props
}: Partial<WorkspaceCreatorProps & { isDashboardAdmin: boolean }>) => {
}: Partial<WorkspaceCreatorProps & { isDashboardAdmin: boolean; dataSourceEnabled?: boolean }>) => {
const { Provider } = createOpenSearchDashboardsReactContext({
...mockCoreStart,
...{
Expand Down Expand Up @@ -153,7 +154,7 @@ const WorkspaceCreator = ({
}),
},
},
dataSourceManagement: {},
dataSourceManagement: dataSourceEnabled ? {} : undefined,
navigationUI: {
HeaderControl: () => null,
},
Expand Down Expand Up @@ -374,7 +375,7 @@ describe('WorkspaceCreator', () => {
value: 600,
});
const { getByTestId, getAllByText, getByText } = render(
<WorkspaceCreator isDashboardAdmin={true} />
<WorkspaceCreator isDashboardAdmin={true} dataSourceEnabled />
);

// Ensure workspace create form rendered
Expand Down Expand Up @@ -432,7 +433,7 @@ describe('WorkspaceCreator', () => {
value: 600,
});
const { getByTestId, getAllByText, getByText } = render(
<WorkspaceCreator isDashboardAdmin={true} />
<WorkspaceCreator isDashboardAdmin={true} dataSourceEnabled />
);

// Ensure workspace create form rendered
Expand Down Expand Up @@ -492,6 +493,10 @@ describe('WorkspaceCreator', () => {
await waitFor(() => {
expect(getByTestId('workspaceForm-bottomBar-createButton')).toBeInTheDocument();
});
const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText');
fireEvent.input(nameInput, {
target: { value: 'test workspace name' },
});
fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton'));
expect(workspaceClientCreate).toHaveBeenCalledTimes(1);

Expand All @@ -512,6 +517,10 @@ describe('WorkspaceCreator', () => {
await waitFor(() => {
expect(getByTestId('workspaceForm-bottomBar-createButton')).toBeInTheDocument();
});
const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText');
fireEvent.input(nameInput, {
target: { value: 'test workspace name' },
});
fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton'));
jest.useFakeTimers();
jest.runAllTimers();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => {
color: euiPaletteColorBlind()[0],
...(defaultSelectedUseCase
? {
name: defaultSelectedUseCase.title,
features: [getUseCaseFeatureConfig(defaultSelectedUseCase.id)],
}
: {}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ export const WorkspaceCreatorForm = (props: WorkspaceCreatorFormProps) => {
formId={formId}
application={application}
isSubmitting={props.isSubmitting}
dataSourceEnabled={!!isDataSourceEnabled}
/>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { WorkspaceFormSummaryPanel, FieldSummaryItem } from './workspace_form_su
import { RightSidebarScrollField } from './utils';
import { WorkspacePermissionItemType } from '../workspace_form';
import { applicationServiceMock } from '../../../../../../src/core/public/mocks';
import { DataSourceConnectionType } from '../../../common/types';
import { WorkspacePermissionMode } from '../../../common/constants';

describe('WorkspaceFormSummaryPanel', () => {
const formData = {
Expand All @@ -18,28 +20,43 @@ describe('WorkspaceFormSummaryPanel', () => {
description: 'This is a test workspace',
color: '#000000',
selectedDataSourceConnections: [
{ id: 'data-source-1', name: 'Data Source 1' },
{ id: 'data-source-2', name: 'Data Source 2' },
{ id: 'data-source-3', name: 'Data Source 3' },
{
id: 'data-source-1',
name: 'Data Source 1',
type: '',
connectionType: DataSourceConnectionType.OpenSearchConnection,
},
{
id: 'data-source-2',
name: 'Data Source 2',
type: '',
connectionType: DataSourceConnectionType.OpenSearchConnection,
},
{
id: 'data-source-3',
name: 'Data Source 3',
type: '',
connectionType: DataSourceConnectionType.OpenSearchConnection,
},
],
permissionSettings: [
{
id: 1,
type: WorkspacePermissionItemType.User,
userId: 'user1',
modes: ['library_write', 'write'],
modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write],
},
{
id: 2,
type: WorkspacePermissionItemType.Group,
group: 'group1',
modes: ['library_read', 'read'],
modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read],
},
{
id: 3,
type: WorkspacePermissionItemType.User,
userId: 'user2',
modes: ['library_write', 'read'],
modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Read],
},
],
};
Expand Down Expand Up @@ -71,6 +88,7 @@ describe('WorkspaceFormSummaryPanel', () => {
formId="id"
application={applicationMock}
isSubmitting={false}
dataSourceEnabled
/>
);

Expand Down Expand Up @@ -107,6 +125,7 @@ describe('WorkspaceFormSummaryPanel', () => {
formId="id"
application={applicationMock}
isSubmitting={false}
dataSourceEnabled
/>
);

Expand Down Expand Up @@ -148,6 +167,7 @@ describe('WorkspaceFormSummaryPanel', () => {
formId="id"
application={applicationMock}
isSubmitting={false}
dataSourceEnabled
/>
);
expect(screen.getByText('user1')).toBeInTheDocument();
Expand All @@ -161,6 +181,21 @@ describe('WorkspaceFormSummaryPanel', () => {
fireEvent.click(screen.getByText('Show less'));
expect(screen.queryByText('user2')).toBeNull();
});

it('should hide "Data sources" if data source not enabled', () => {
render(
<WorkspaceFormSummaryPanel
formData={formData}
availableUseCases={availableUseCases}
permissionEnabled
formId="id"
application={applicationMock}
isSubmitting={false}
dataSourceEnabled={false}
/>
);
expect(screen.queryByText('Data sources')).toBeNull();
});
});

describe('FieldSummaryItem', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ interface WorkspaceFormSummaryPanelProps {
formId: string;
application: ApplicationStart;
isSubmitting: boolean;
dataSourceEnabled: boolean;
}

export const WorkspaceFormSummaryPanel = ({
Expand All @@ -174,6 +175,7 @@ export const WorkspaceFormSummaryPanel = ({
formId,
application,
isSubmitting,
dataSourceEnabled,
}: WorkspaceFormSummaryPanelProps) => {
const useCase = availableUseCases.find((item) => item.id === formData.useCase);
const userAndGroups: UserAndGroups[] = formData.permissionSettings.flatMap((setting) => {
Expand Down Expand Up @@ -213,18 +215,20 @@ export const WorkspaceFormSummaryPanel = ({
<FieldSummaryItem field={RightSidebarScrollField.UseCase}>
{useCase && <EuiText size="xs">{useCase.title}</EuiText>}
</FieldSummaryItem>
<FieldSummaryItem field={RightSidebarScrollField.DataSource}>
{formData.selectedDataSourceConnections.length > 0 && (
<ExpandableTextList
items={formData.selectedDataSourceConnections.map((connection) => (
<ul key={connection.id} style={{ marginBottom: 0 }}>
<li>{connection.name}</li>
</ul>
))}
collapseDisplayCount={3}
/>
)}
</FieldSummaryItem>
{dataSourceEnabled && (
<FieldSummaryItem field={RightSidebarScrollField.DataSource}>
{formData.selectedDataSourceConnections.length > 0 && (
<ExpandableTextList
items={formData.selectedDataSourceConnections.map((connection) => (
<ul key={connection.id} style={{ marginBottom: 0 }}>
<li>{connection.name}</li>
</ul>
))}
collapseDisplayCount={3}
/>
)}
</FieldSummaryItem>
)}
{permissionEnabled && (
<FieldSummaryItem bottomGap={false} field={RightSidebarScrollField.Collaborators}>
{userAndGroups.length > 0 && (
Expand All @@ -240,6 +244,7 @@ export const WorkspaceFormSummaryPanel = ({
formId={formId}
application={application}
isSubmitting={isSubmitting}
dataSourceEnabled={dataSourceEnabled}
/>
</EuiCard>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ interface DataSourceConnectionTableProps {
onUnlinkDataSource: (dataSources: DataSourceConnection) => void;
onSelectionChange: (selections: DataSourceConnection[]) => void;
dataSourceConnections: DataSourceConnection[];
tableProps?: Pick<EuiInMemoryTableProps<DataSourceConnection>, 'pagination' | 'search'>;
tableProps?: Pick<
EuiInMemoryTableProps<DataSourceConnection>,
'pagination' | 'search' | 'message'
>;
}

export const DataSourceConnectionTable = forwardRef<
Expand Down
Loading
Loading