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] feat: only allow essential use case when creating workspace if all data sources are serverless #7684

Merged
merged 1 commit into from
Aug 12, 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/7612.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feat:
- Only allow essential use case when creating workspace if all data sources are serverless ([#7612](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7612))
1 change: 1 addition & 0 deletions src/plugins/data_source/common/data_sources/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export enum AuthType {
SigV4 = 'sigv4',
}

// src/plugins/workspace/public/utils.ts Workspace plugin depends on this to do use case limitation.
export enum SigV4ServiceName {
OpenSearch = 'es',
OpenSearchServerless = 'aoss',
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/workspace/opensearch_dashboards.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@
"opensearchDashboardsReact"
],
"optionalPlugins": ["savedObjectsManagement","management","dataSourceManagement","contentManagement"],
"requiredBundles": ["opensearchDashboardsReact", "home"]
"requiredBundles": ["opensearchDashboardsReact", "home","dataSource"]
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ export const WorkspaceDetailForm = (props: WorkspaceFormProps) => {
onChange={handleUseCaseChange}
formErrors={formErrors}
availableUseCases={availableUseCases}
savedObjects={savedObjects}
operationType={operationType}
/>
</FormGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => {
onChange={handleUseCaseChange}
formErrors={formErrors}
availableUseCases={availableUseCases}
savedObjects={savedObjects}
operationType={operationType}
/>
</EuiPanel>
<EuiSpacer />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,23 @@
*/

import React from 'react';
import { fireEvent, render } from '@testing-library/react';
import { fireEvent, render, waitFor } from '@testing-library/react';
import { WORKSPACE_USE_CASES } from '../../../common/constants';
import { WorkspaceUseCase, WorkspaceUseCaseProps } from './workspace_use_case';
import { WorkspaceFormErrors } from './types';
import { coreMock } from '../../../../../core/public/mocks';
import { WorkspaceOperationType } from './constants';
import { getIsOnlyAllowEssentialUseCase } from '../../utils';

jest.mock('../../utils', () => ({
getIsOnlyAllowEssentialUseCase: jest.fn().mockResolvedValue(false),
}));
const mockCoreStart = coreMock.createStart();

const setup = (options?: Partial<WorkspaceUseCaseProps>) => {
const onChangeMock = jest.fn();
const formErrors: WorkspaceFormErrors = {};
const savedObjects = mockCoreStart.savedObjects;
const renderResult = render(
<WorkspaceUseCase
availableUseCases={[
Expand All @@ -29,6 +38,8 @@ const setup = (options?: Partial<WorkspaceUseCaseProps>) => {
value=""
onChange={onChangeMock}
formErrors={formErrors}
operationType={WorkspaceOperationType.Create}
savedObjects={savedObjects}
{...options}
/>
);
Expand Down Expand Up @@ -63,4 +74,13 @@ describe('WorkspaceUseCase', () => {
fireEvent.click(renderResult.getByText('Observability'));
expect(onChangeMock).not.toHaveBeenCalled();
});
it('should only display essential use case when creating workspace if getIsOnlyAllowEssentialUseCase returns true', async () => {
(getIsOnlyAllowEssentialUseCase as jest.Mock).mockResolvedValue(true);

const { renderResult } = setup();
await waitFor(() => {
expect(renderResult.queryByText('Essentials')).toBeInTheDocument();
expect(renderResult.queryByText('Observability')).not.toBeInTheDocument();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import React, { useCallback } from 'react';
import React, { useCallback, useState, useEffect, useMemo } from 'react';
import { i18n } from '@osd/i18n';
import {
EuiCheckableCard,
Expand All @@ -17,6 +17,9 @@ import { DEFAULT_NAV_GROUPS } from '../../../../../core/public';
import { WorkspaceUseCase as WorkspaceUseCaseObject } from '../../types';
import { WorkspaceFormErrors } from './types';
import './workspace_use_case.scss';
import type { SavedObjectsStart } from '../../../../../core/public';
import { getIsOnlyAllowEssentialUseCase } from '../../utils';
import { WorkspaceOperationType } from './constants';

interface WorkspaceUseCaseCardProps {
id: string;
Expand Down Expand Up @@ -54,21 +57,48 @@ const WorkspaceUseCaseCard = ({
);
};

type AvailableUseCase = Pick<WorkspaceUseCaseObject, 'id' | 'title' | 'description' | 'systematic'>;

export interface WorkspaceUseCaseProps {
value: string | undefined;
onChange: (newValue: string) => void;
formErrors: WorkspaceFormErrors;
availableUseCases: Array<
Pick<WorkspaceUseCaseObject, 'id' | 'title' | 'description' | 'systematic'>
>;
availableUseCases: AvailableUseCase[];
savedObjects: SavedObjectsStart;
operationType: WorkspaceOperationType;
}

export const WorkspaceUseCase = ({
value,
onChange,
formErrors,
availableUseCases,
savedObjects,
operationType,
}: WorkspaceUseCaseProps) => {
const [isOnlyAllowEssential, setIsOnlyAllowEssential] = useState(false);

useEffect(() => {
if (operationType === WorkspaceOperationType.Create) {
getIsOnlyAllowEssentialUseCase(savedObjects.client).then((result: boolean) => {
setIsOnlyAllowEssential(result);
});
}
}, [savedObjects, operationType]);

const displayedUseCases = useMemo(() => {
let allAvailableUseCases = availableUseCases
.filter((item) => !item.systematic)
.concat(DEFAULT_NAV_GROUPS.all);
// When creating and isOnlyAllowEssential is true, only display essential use case
if (isOnlyAllowEssential && operationType === WorkspaceOperationType.Create) {
allAvailableUseCases = allAvailableUseCases.filter(
(item) => item.id === DEFAULT_NAV_GROUPS.essentials.id
);
}
return allAvailableUseCases;
}, [availableUseCases, isOnlyAllowEssential, operationType]);

return (
<EuiCompressedFormRow
label={i18n.translate('workspace.form.workspaceUseCase.name.label', {
Expand All @@ -79,20 +109,17 @@ export const WorkspaceUseCase = ({
fullWidth
>
<EuiFlexGroup>
{availableUseCases
.filter((item) => !item.systematic)
.concat(DEFAULT_NAV_GROUPS.all)
.map(({ id, title, description }) => (
<EuiFlexItem key={id}>
<WorkspaceUseCaseCard
id={id}
title={title}
description={description}
checked={value === id}
onChange={onChange}
/>
</EuiFlexItem>
))}
{displayedUseCases.map(({ id, title, description }) => (
<EuiFlexItem key={id}>
<WorkspaceUseCaseCard
id={id}
title={title}
description={description}
checked={value === id}
onChange={onChange}
/>
</EuiFlexItem>
))}
</EuiFlexGroup>
</EuiCompressedFormRow>
);
Expand Down
57 changes: 55 additions & 2 deletions src/plugins/workspace/public/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ import {
isEqualWorkspaceUseCase,
USE_CASE_PREFIX,
prependWorkspaceToBreadcrumbs,
getIsOnlyAllowEssentialUseCase,
} from './utils';
import { WorkspaceAvailability } from '../../../core/public';
import { coreMock } from '../../../core/public/mocks';
import { WORKSPACE_DETAIL_APP_ID, WORKSPACE_USE_CASES } from '../common/constants';
import { SigV4ServiceName } from '../../../plugins/data_source/common/data_sources';

const startMock = coreMock.createStart();
const STATIC_USE_CASES = [
Expand Down Expand Up @@ -379,15 +381,16 @@ describe('workspace utils: getDataSourcesList', () => {
{
id: 'id1',
get: () => {
return 'title1';
return 'mock_value';
},
},
],
});
expect(await getDataSourcesList(mockedSavedObjectClient, [])).toStrictEqual([
{
id: 'id1',
title: 'title1',
title: 'mock_value',
auth: 'mock_value',
},
]);
});
Expand All @@ -398,6 +401,56 @@ describe('workspace utils: getDataSourcesList', () => {
});
});

describe('workspace utils: getIsOnlyAllowEssentialUseCase', () => {
const mockedSavedObjectClient = startMock.savedObjects.client;

it('should return true when all data sources are serverless', async () => {
mockedSavedObjectClient.find = jest.fn().mockResolvedValue({
savedObjects: [
{
id: 'id1',
get: () => {
return {
credentials: {
service: SigV4ServiceName.OpenSearchServerless,
},
};
},
},
],
});
expect(await getIsOnlyAllowEssentialUseCase(mockedSavedObjectClient)).toBe(true);
});

it('should return false when not all data sources are serverless', async () => {
mockedSavedObjectClient.find = jest.fn().mockResolvedValue({
savedObjects: [
{
id: 'id1',
get: () => {
return {
credentials: {
service: SigV4ServiceName.OpenSearchServerless,
},
};
},
},
{
id: 'id2',
get: () => {
return {
credentials: {
service: SigV4ServiceName.OpenSearch,
},
};
},
},
],
});
expect(await getIsOnlyAllowEssentialUseCase(mockedSavedObjectClient)).toBe(false);
});
});

describe('workspace utils: convertNavGroupToWorkspaceUseCase', () => {
it('should convert nav group to consistent workspace use case', () => {
expect(
Expand Down
16 changes: 15 additions & 1 deletion src/plugins/workspace/public/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
} from '../../../core/public';
import { DEFAULT_SELECTED_FEATURES_IDS, WORKSPACE_DETAIL_APP_ID } from '../common/constants';
import { WorkspaceUseCase } from './types';
import { SigV4ServiceName } from '../../../plugins/data_source/common/data_sources';

export const USE_CASE_PREFIX = 'use-case-';

Expand Down Expand Up @@ -202,7 +203,7 @@
return client
.find({
type: 'data-source',
fields: ['id', 'title'],
fields: ['id', 'title', 'auth'],
perPage: 10000,
workspaces,
})
Expand All @@ -212,9 +213,11 @@
return objects.map((source) => {
const id = source.id;
const title = source.get('title');
const auth = source.get('auth');
return {
id,
title,
auth,
};
});
} else {
Expand All @@ -223,6 +226,17 @@
});
};

// If all connected data sources are serverless, will only allow to select essential use case.
export const getIsOnlyAllowEssentialUseCase = async (client: SavedObjectsStart['client']) => {
const allDataSources = await getDataSourcesList(client, ['*']);
if (allDataSources.length > 0) {
return allDataSources.every(
(ds) => ds?.auth?.credentials?.service === SigV4ServiceName.OpenSearchServerless
);
}
return false;

Check warning on line 237 in src/plugins/workspace/public/utils.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/workspace/public/utils.ts#L237

Added line #L237 was not covered by tests
};

export const convertNavGroupToWorkspaceUseCase = ({
id,
title,
Expand Down
Loading