Skip to content

Commit

Permalink
[Workspace]Restrict at least one data source in workspace creation pa…
Browse files Browse the repository at this point in the history
…ge (#8461) (#8468)

* Restrict at least one data sources for workspace creation



* Update empty data sources panel UI



* Changeset file for PR #8461 created/updated

* Fix wrong testing id for data source empty state



* Remove populate workspace name



---------



(cherry picked from commit ec4b9d7)

Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Oct 4, 2024
1 parent db1bf09 commit f82075a
Show file tree
Hide file tree
Showing 11 changed files with 226 additions and 52 deletions.
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

0 comments on commit f82075a

Please sign in to comment.