Skip to content

Commit

Permalink
[Workspace]Add "All use case" option to workspace form (#7318) (#7338)
Browse files Browse the repository at this point in the history
* Add all use case option



* Changeset file for PR #7318 created/updated

* Changeset file for PR #7318 created/updated

* Changeset file for PR #7318 created/updated

* Fix use case selection



* All use case can accessible any app and add comment



* Remove duplicate style import



---------



(cherry picked from commit 5741fd7)

Signed-off-by: Lin Wang <wonglam@amazon.com>
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 Jul 21, 2024
1 parent 4b3bc43 commit 47e84ed
Show file tree
Hide file tree
Showing 12 changed files with 122 additions and 70 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/7318.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feat:
- [Workspace]Add "All use case" option to workspace form ([#7318](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7318))
Original file line number Diff line number Diff line change
Expand Up @@ -127,4 +127,17 @@ describe('useWorkspaceForm', () => {
})
);
});
it('should update selected use case', () => {
const { renderResult } = setup({
id: 'foo',
name: 'test-workspace-name',
features: ['use-case-observability'],
});

expect(renderResult.result.current.formData.useCase).toBe('observability');
act(() => {
renderResult.result.current.handleUseCaseChange('search');
});
expect(renderResult.result.current.formData.useCase).toBe('search');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import {

import { useApplications } from '../../hooks';
import {
getFirstUseCaseOfFeatureConfigs,
getUseCaseFeatureConfig,
getUseCaseFromFeatureConfig,
isUseCaseFeatureConfig,
} from '../../utils';
import { DataSource } from '../../../common/types';
Expand All @@ -30,8 +30,6 @@ import { WorkspacePermissionItemType } from './constants';

const workspaceHtmlIdGenerator = htmlIdGenerator();

const isNotNull = <T extends unknown>(value: T | null): value is T => !!value;

export const useWorkspaceForm = ({
application,
defaultValues,
Expand All @@ -51,10 +49,9 @@ export const useWorkspaceForm = ({
const [featureConfigs, setFeatureConfigs] = useState(
appendDefaultFeatureIds(defaultValues?.features ?? [])
);
const selectedUseCases = useMemo(
() => featureConfigs.map(getUseCaseFromFeatureConfig).filter(isNotNull),
[featureConfigs]
);
const selectedUseCase = useMemo(() => getFirstUseCaseOfFeatureConfigs(featureConfigs), [
featureConfigs,
]);
const [permissionSettings, setPermissionSettings] = useState<
Array<Pick<WorkspacePermissionSetting, 'id'> & Partial<WorkspacePermissionSetting>>
>(initialPermissionSettingsRef.current);
Expand All @@ -72,7 +69,7 @@ export const useWorkspaceForm = ({
name,
description,
features: featureConfigs,
useCases: selectedUseCases,
useCase: selectedUseCase,
color,
permissionSettings,
selectedDataSources,
Expand All @@ -92,14 +89,14 @@ export const useWorkspaceForm = ({
formIdRef.current = workspaceHtmlIdGenerator();
}

const handleUseCasesChange = useCallback(
(newUseCases: string[]) => {
const handleUseCaseChange = useCallback(
(newUseCase: string) => {
setFeatureConfigs((previousFeatureConfigs) => {
return [
...previousFeatureConfigs.filter(
(featureConfig) => !isUseCaseFeatureConfig(featureConfig)
),
...newUseCases.map((useCaseItem) => getUseCaseFeatureConfig(useCaseItem)),
getUseCaseFeatureConfig(newUseCase),
];
});
},
Expand Down Expand Up @@ -157,7 +154,7 @@ export const useWorkspaceForm = ({
numberOfChanges,
handleFormSubmit,
handleColorChange,
handleUseCasesChange,
handleUseCaseChange,
handleNameInputChange,
setPermissionSettings,
setSelectedDataSources,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export const WorkspaceDetailForm = (props: WorkspaceFormProps) => {
numberOfChanges,
handleFormSubmit,
handleColorChange,
handleUseCasesChange,
handleUseCaseChange,
setPermissionSettings,
handleNameInputChange,
setSelectedDataSources,
Expand Down Expand Up @@ -109,8 +109,8 @@ export const WorkspaceDetailForm = (props: WorkspaceFormProps) => {

<FormGroup title={workspaceUseCaseTitle}>
<WorkspaceUseCase
value={formData.useCases}
onChange={handleUseCasesChange}
value={formData.useCase}
onChange={handleUseCaseChange}
formErrors={formErrors}
availableUseCases={availableUseCases}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => {
numberOfChanges,
handleFormSubmit,
handleColorChange,
handleUseCasesChange,
handleUseCaseChange,
handleNameInputChange,
setPermissionSettings,
setSelectedDataSources,
Expand Down Expand Up @@ -85,8 +85,8 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => {
</EuiTitle>
<EuiSpacer size="s" />
<WorkspaceUseCase
value={formData.useCases}
onChange={handleUseCasesChange}
value={formData.useCase}
onChange={handleUseCaseChange}
formErrors={formErrors}
availableUseCases={availableUseCases}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@ const setup = (options?: Partial<WorkspaceUseCaseProps>) => {
id: 'system-use-case',
title: 'System use case',
description: 'System use case description',
features: [],
systematic: true,
},
]}
value={[]}
value=""
onChange={onChangeMock}
formErrors={formErrors}
{...options}
Expand All @@ -49,19 +48,19 @@ describe('WorkspaceUseCase', () => {
expect(renderResult.getByText('Search')).toBeInTheDocument();
});

it('should call onChange with new added use case', () => {
it('should call onChange with new checked use case', () => {
const { renderResult, onChangeMock } = setup();

expect(onChangeMock).not.toHaveBeenCalled();
fireEvent.click(renderResult.getByText('Observability'));
expect(onChangeMock).toHaveBeenLastCalledWith(['observability']);
expect(onChangeMock).toHaveBeenLastCalledWith('observability');
});

it('should call onChange without removed use case', () => {
const { renderResult, onChangeMock } = setup({ value: ['observability'] });
it('should not call onChange after checked use case clicked', () => {
const { renderResult, onChangeMock } = setup({ value: 'observability' });

expect(onChangeMock).not.toHaveBeenCalled();
fireEvent.click(renderResult.getByText('Observability'));
expect(onChangeMock).toHaveBeenLastCalledWith([]);
expect(onChangeMock).not.toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import React, { useCallback } from 'react';
import { i18n } from '@osd/i18n';
import { EuiCheckableCard, EuiFlexGroup, EuiFlexItem, EuiFormRow, EuiText } from '@elastic/eui';

import { DEFAULT_NAV_GROUPS } from '../../../../../core/public';
import { WorkspaceUseCase as WorkspaceUseCaseObject } from '../../types';
import { WorkspaceFormErrors } from './types';
import './workspace_use_case.scss';
Expand All @@ -31,7 +33,7 @@ const WorkspaceUseCaseCard = ({
return (
<EuiCheckableCard
id={id}
checkableType="checkbox"
checkableType="radio"
style={{ height: '100%' }}
label={title}
checked={checked}
Expand All @@ -47,10 +49,12 @@ const WorkspaceUseCaseCard = ({
};

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

export const WorkspaceUseCase = ({
Expand All @@ -59,17 +63,6 @@ export const WorkspaceUseCase = ({
formErrors,
availableUseCases,
}: WorkspaceUseCaseProps) => {
const handleCardChange = useCallback(
(id: string) => {
if (!value.includes(id)) {
onChange([...value, id]);
return;
}
onChange(value.filter((item) => item !== id));
},
[value, onChange]
);

return (
<EuiFormRow
label={i18n.translate('workspace.form.workspaceUseCase.name.label', {
Expand All @@ -82,14 +75,15 @@ export const WorkspaceUseCase = ({
<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.includes(id)}
onChange={handleCardChange}
checked={value === id}
onChange={onChange}
/>
</EuiFlexItem>
))}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ jest.mock('../delete_workspace_modal', () => ({

function getWrapWorkspaceListInContext(
workspaceList = [
{ id: 'id1', name: 'name1', features: [] },
{ id: 'id1', name: 'name1', features: ['use-case-all'] },
{ id: 'id2', name: 'name2' },
{ id: 'id3', name: 'name3', features: ['use-case-observability'] },
]
Expand Down Expand Up @@ -69,6 +69,7 @@ describe('WorkspaceList', () => {
expect(getByText('name2')).toBeInTheDocument();

// should display use case
expect(getByText('All use case')).toBeInTheDocument();
expect(getByText('Observability')).toBeInTheDocument();
});
it('should be able to apply debounce search after input', async () => {
Expand Down
23 changes: 10 additions & 13 deletions src/plugins/workspace/public/components/workspace_list/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
import useObservable from 'react-use/lib/useObservable';
import { BehaviorSubject, of } from 'rxjs';
import { i18n } from '@osd/i18n';
import { debounce } from '../../../../../core/public';
import { debounce, DEFAULT_NAV_GROUPS } from '../../../../../core/public';
import { WorkspaceAttribute } from '../../../../../core/public';
import { useOpenSearchDashboards } from '../../../../../plugins/opensearch_dashboards_react/public';
import { navigateToWorkspaceDetail } from '../utils/workspace';
Expand All @@ -26,7 +26,7 @@ import { WORKSPACE_CREATE_APP_ID } from '../../../common/constants';

import { cleanWorkspaceId } from '../../../../../core/public';
import { DeleteWorkspaceModal } from '../delete_workspace_modal';
import { getUseCaseFromFeatureConfig } from '../../utils';
import { getFirstUseCaseOfFeatureConfigs, getUseCaseFromFeatureConfig } from '../../utils';
import { WorkspaceUseCase } from '../../types';

const WORKSPACE_LIST_PAGE_DESCRIPTION = i18n.translate('workspace.list.description', {
Expand Down Expand Up @@ -108,17 +108,14 @@ export const WorkspaceList = ({ registeredUseCases$ }: WorkspaceListProps) => {
if (!features || features.length === 0) {
return '';
}
const results: string[] = [];
features.forEach((featureConfig) => {
const useCaseId = getUseCaseFromFeatureConfig(featureConfig);
if (useCaseId) {
const useCase = registeredUseCases?.find(({ id }) => id === useCaseId);
if (useCase) {
results.push(useCase.title);
}
}
});
return results.join(', ');
const useCaseId = getFirstUseCaseOfFeatureConfigs(features);
const useCase =
useCaseId === DEFAULT_NAV_GROUPS.all.id
? DEFAULT_NAV_GROUPS.all
: registeredUseCases?.find(({ id }) => id === useCaseId);
if (useCase) {
return useCase.title;
}
},
},
{
Expand Down
13 changes: 13 additions & 0 deletions src/plugins/workspace/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
NavGroupStatus,
DEFAULT_NAV_GROUPS,
NavGroupType,
ALL_USE_CASE_ID,
} from '../../../core/public';
import {
WORKSPACE_FATAL_ERROR_APP_ID,
Expand All @@ -38,6 +39,7 @@ import { getWorkspaceColumn } from './components/workspace_column';
import { DataSourceManagementPluginSetup } from '../../../plugins/data_source_management/public';
import {
filterWorkspaceConfigurableApps,
getFirstUseCaseOfFeatureConfigs,
isAppAccessibleInWorkspace,
isNavGroupInFeatureConfigs,
} from './utils';
Expand Down Expand Up @@ -119,9 +121,20 @@ export class WorkspacePlugin implements Plugin<{}, {}, WorkspacePluginSetupDeps>
this.currentWorkspaceSubscription = currentWorkspace$.subscribe((currentWorkspace) => {
if (currentWorkspace) {
this.navGroupUpdater$.next((navGroup) => {
/**
* The following logic determines whether a navigation group should be hidden or not based on the workspace's feature configurations.
* It checks the following conditions:
* 1. The navigation group is not a system-level group (system groups are always visible).
* 2. The current workspace has feature configurations set up.
* 3. The current workspace's use case it not "All use case".
* 4. The current navigation group is not included in the feature configurations of the workspace.
*
* If all these conditions are true, it means that the navigation group should be hidden.
*/
if (
navGroup.type !== NavGroupType.SYSTEM &&
currentWorkspace.features &&
getFirstUseCaseOfFeatureConfigs(currentWorkspace.features) !== ALL_USE_CASE_ID &&
!isNavGroupInFeatureConfigs(navGroup.id, currentWorkspace.features)
) {
return {
Expand Down
25 changes: 20 additions & 5 deletions src/plugins/workspace/public/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,18 @@ describe('workspace utils: featureMatchesConfig', () => {

it('should match features include by any use cases', () => {
const match = featureMatchesConfig(
['use-case-observability', 'use-case-analytics'],
['use-case-observability', 'use-case-search'],
STATIC_USE_CASES
);
expect(match({ id: 'dashboards' })).toBe(true);
expect(match({ id: 'observability-traces' })).toBe(true);
expect(match({ id: 'alerting' })).toBe(true);

/**
* The searchRelevance is a feature under search use case. Since each workspace only can be a specific use case,
* the feature matches will use first use case to check if features exists. The observability doesn't have
* searchRelevance feature, it will return false.
*/
expect(match({ id: 'searchRelevance' })).toBe(false);
expect(match({ id: 'not-in-any-use-case' })).toBe(false);
});
});
Expand Down Expand Up @@ -240,6 +246,15 @@ describe('workspace utils: isAppAccessibleInWorkspace', () => {
)
).toBe(true);
});
it('any app is accessible when workspace is all use case', () => {
expect(
isAppAccessibleInWorkspace(
{ id: 'any_app', title: 'Any app', mount: jest.fn() },
{ id: 'workspace_id', name: 'workspace name', features: ['use-case-all'] },
STATIC_USE_CASES
)
).toBe(true);
});
});

describe('workspace utils: filterWorkspaceConfigurableApps', () => {
Expand Down Expand Up @@ -309,11 +324,11 @@ describe('workspace utils: filterWorkspaceConfigurableApps', () => {

describe('workspace utils: isFeatureIdInsideUseCase', () => {
it('should return false for invalid use case', () => {
expect(isFeatureIdInsideUseCase('discover', 'use-case-invalid', [])).toBe(false);
expect(isFeatureIdInsideUseCase('discover', 'invalid', [])).toBe(false);
});
it('should return false if feature not in use case', () => {
expect(
isFeatureIdInsideUseCase('discover', 'use-case-foo', [
isFeatureIdInsideUseCase('discover', 'foo', [
{
id: 'foo',
title: 'Foo',
Expand All @@ -325,7 +340,7 @@ describe('workspace utils: isFeatureIdInsideUseCase', () => {
});
it('should return true if feature id exists in use case', () => {
expect(
isFeatureIdInsideUseCase('discover', 'use-case-foo', [
isFeatureIdInsideUseCase('discover', 'foo', [
{
id: 'foo',
title: 'Foo',
Expand Down
Loading

0 comments on commit 47e84ed

Please sign in to comment.