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

[Cases] Delete custom field #167167

Merged
Show file tree
Hide file tree
Changes from 7 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
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import userEvent from '@testing-library/user-event';

import { ConfigureCases } from '.';
import { noUpdateCasesPermissions, TestProviders, createAppMockRenderer } from '../../common/mock';
import { customFieldsConfigurationMock } from '../../containers/mock';
import type { AppMockRenderer } from '../../common/mock';
import { Connectors } from './connectors';
import { ClosureOptions } from './closure_options';
Expand Down Expand Up @@ -603,10 +604,16 @@ describe('ConfigureCases', () => {

describe('custom fields', () => {
let appMockRender: AppMockRenderer;
let persistCaseConfigure: jest.Mock;

beforeEach(() => {
jest.clearAllMocks();
appMockRender = createAppMockRenderer();
persistCaseConfigure = jest.fn();
usePersistConfigurationMock.mockImplementation(() => ({
...usePersistConfigurationMockResponse,
mutate: persistCaseConfigure,
}));
});

it('renders custom field group when no custom fields available', () => {
Expand Down Expand Up @@ -654,26 +661,11 @@ describe('ConfigureCases', () => {
});

it('renders multiple custom field when available', () => {
const customFieldsMock: CustomFieldsConfiguration = [
{
key: 'random_custom_key',
label: 'Summary',
type: CustomFieldTypes.TEXT,
required: true,
},
{
key: 'random_custom_key_2',
label: 'Maintenance',
type: CustomFieldTypes.TOGGLE,
required: false,
},
];

useGetCaseConfigurationMock.mockImplementation(() => ({
...useCaseConfigureResponse,
data: {
...useCaseConfigureResponse.data,
customFields: customFieldsMock,
customFields: customFieldsConfigurationMock,
currentConfiguration: {
connector: {
id: 'resilient-2',
Expand All @@ -690,13 +682,61 @@ describe('ConfigureCases', () => {

const droppable = screen.getByTestId('droppable');

for (const field of customFieldsMock) {
for (const field of customFieldsConfigurationMock) {
expect(
within(droppable).getByTestId(`custom-field-${field.label}-${field.type}`)
).toBeInTheDocument();
}
});

it('deletes a custom field correctly', async () => {
useGetCaseConfigurationMock.mockImplementation(() => ({
...useCaseConfigureResponse,
data: {
...useCaseConfigureResponse.data,
customFields: customFieldsConfigurationMock,
currentConfiguration: {
connector: {
id: 'resilient-2',
name: 'unchanged',
type: ConnectorTypes.serviceNowITSM,
fields: null,
},
closureType: 'close-by-user',
},
},
}));

appMockRender.render(<ConfigureCases />);

const droppable = screen.getByTestId('droppable');

userEvent.click(
within(droppable).getByTestId(`${customFieldsConfigurationMock[0].key}-custom-field-delete`)
);

await waitFor(() => {
expect(screen.getByTestId('confirm-delete-custom-field-modal')).toBeInTheDocument();
});

userEvent.click(screen.getByText('Delete'));

await waitFor(() => {
expect(persistCaseConfigure).toHaveBeenCalledWith({
connector: {
id: 'none',
name: 'none',
type: ConnectorTypes.none,
fields: null,
},
closureType: 'close-by-user',
customFields: [{ ...customFieldsConfigurationMock[1] }],
id: '',
version: '',
});
});
});

it('opens fly out for when click on add field', async () => {
appMockRender.render(<ConfigureCases />);

Expand Down Expand Up @@ -729,7 +769,30 @@ describe('ConfigureCases', () => {

userEvent.click(screen.getByTestId('add-custom-field-flyout-save'));

expect(await screen.findByTestId('custom-fields-form-group')).toBeInTheDocument();
await waitFor(() => {
expect(persistCaseConfigure).toHaveBeenCalledWith({
connector: {
id: 'none',
name: 'none',
type: ConnectorTypes.none,
fields: null,
},
closureType: 'close-by-user',
customFields: [
...customFieldsConfigurationMock,
{
key: expect.anything(),
label: 'Summary',
type: CustomFieldTypes.TEXT,
required: false,
},
],
id: '',
version: '',
});
});

expect(screen.getByTestId('custom-fields-form-group')).toBeInTheDocument();
expect(screen.queryByTestId('add-custom-field-flyout')).not.toBeInTheDocument();
});
});
Expand Down
23 changes: 23 additions & 0 deletions x-pack/plugins/cases/public/components/configure_cases/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,28 @@ export const ConfigureCases: React.FC = React.memo(() => {
setAddFieldFlyoutVisibility(true);
}, [setAddFieldFlyoutVisibility]);

const onDeleteCustomField = useCallback(
(key: string) => {
const remainingCustomFields = customFields.filter((field) => field.key !== key);

persistCaseConfigure({
connector,
customFields: [...remainingCustomFields],
id: configurationId,
version: configurationVersion,
closureType,
});
},
[
closureType,
configurationId,
configurationVersion,
connector,
customFields,
persistCaseConfigure,
]
);

const onCloseAddFieldFlyout = useCallback(() => {
setAddFieldFlyoutVisibility(false);
}, [setAddFieldFlyoutVisibility]);
Expand Down Expand Up @@ -341,6 +363,7 @@ export const ConfigureCases: React.FC = React.memo(() => {
isLoading={loadingCaseConfigure || isPersistingConfiguration}
disabled={isPersistingConfiguration || loadingCaseConfigure}
handleAddCustomField={onAddCustomFields}
handleDeleteCustomField={onDeleteCustomField}
/>
</EuiFlexItem>
</SectionWrapper>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@

import React from 'react';
import { screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

import type { AppMockRenderer } from '../../common/mock';
import { createAppMockRenderer } from '../../common/mock';
import { FormTestComponent } from '../../common/test_utils';
import { customFieldsConfigurationMock } from '../../containers/mock';
import { CustomFields } from './custom_fields';
import userEvent from '@testing-library/user-event';
import * as i18n from './translations';

describe('CustomFields', () => {
let appMockRender: AppMockRenderer;
Expand All @@ -31,6 +32,7 @@ describe('CustomFields', () => {
</FormTestComponent>
);

expect(screen.getByText(i18n.ADDITIONAL_FIELDS)).toBeInTheDocument();
expect(screen.getByTestId('create-case-custom-fields')).toBeInTheDocument();

for (const item of customFieldsConfigurationMock) {
Expand All @@ -47,6 +49,7 @@ describe('CustomFields', () => {
</FormTestComponent>
);

expect(screen.queryByText(i18n.ADDITIONAL_FIELDS)).not.toBeInTheDocument();
expect(screen.queryAllByTestId('create-custom-field', { exact: false }).length).toEqual(0);
});

Expand Down
17 changes: 16 additions & 1 deletion x-pack/plugins/cases/public/components/create/custom_fields.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,20 @@

import React, { useMemo } from 'react';
import { sortBy } from 'lodash';
import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import { css } from '@emotion/react';
import { EuiFlexGroup, EuiFlexItem, EuiSpacer, EuiText, useEuiTheme } from '@elastic/eui';

import type { CasesConfigurationUI } from '../../../common/ui';
import { builderMap as customFieldsBuilderMap } from '../custom_fields/builder';
import * as i18n from './translations';

interface Props {
isLoading: boolean;
customFieldsConfiguration: CasesConfigurationUI['customFields'];
}

const CustomFieldsComponent: React.FC<Props> = ({ isLoading, customFieldsConfiguration }) => {
const { euiTheme } = useEuiTheme();
const sortedCustomFields = useMemo(
() => sortCustomFieldsByLabel(customFieldsConfiguration),
[customFieldsConfiguration]
Expand All @@ -41,6 +45,17 @@ const CustomFieldsComponent: React.FC<Props> = ({ isLoading, customFieldsConfigu

return (
<EuiFlexGroup direction="column" gutterSize="s">
{customFieldsComponents.length ? (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that the whole component should not be returned if the are no custom fields configured. Maybe we can either return null for the whole component ({customFieldsComponents.length ? ( <EuiFlexGroup direction="column" gutterSize="s">....) or in x-pack/plugins/cases/public/components/create/form.tsx check if customFieldsConfiguration and not render CustomFields if they are empty.

<EuiText
size="m"
css={css`
font-weight: ${euiTheme.font.weight.bold};
`}
>
{i18n.ADDITIONAL_FIELDS}
Copy link
Contributor Author

@js-jankisalvi js-jankisalvi Sep 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed this header from the designs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using an h3 or h4 and playing with the size instead of using CSS?

</EuiText>
) : null}
<EuiSpacer size="xs" />
<EuiFlexItem data-test-subj="create-case-custom-fields">{customFieldsComponents}</EuiFlexItem>
</EuiFlexGroup>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import React, { useCallback, useMemo } from 'react';
import { Form, useForm } from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib';
import { isEmpty } from 'lodash';
import { NONE_CONNECTOR_ID } from '../../../common/constants';
import { CaseSeverity } from '../../../common/types/domain';
import { CaseSeverity, CustomFieldTypes } from '../../../common/types/domain';
import type { FormProps } from './schema';
import { schema } from './schema';
import { getNoneConnector, normalizeActionConnector } from '../configure_cases/utils';
Expand Down Expand Up @@ -106,9 +106,11 @@ export const FormContext: React.FC<Props> = ({

for (const [key, value] of Object.entries(customFields)) {
const configCustomField = customFieldsConfiguration.find((item) => item.key === key);
const fieldValue = isEmpty(value) ? null : [value];

if (configCustomField) {
const fieldValue =
configCustomField.type === CustomFieldTypes.TEXT && isEmpty(value) ? null : [value];
Copy link
Contributor Author

@js-jankisalvi js-jankisalvi Sep 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a bug where you switch the toggle field as on during create case and it still remained off in case view page.
Because it was set to null here.

Copy link
Member

@cnasikas cnasikas Sep 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of checking specifically for a custom field type we can do isEmpty(value) && value !== false ? null : [value]

Copy link
Contributor Author

@js-jankisalvi js-jankisalvi Sep 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check still have issue when toggle field value is true when creating a case, I updated it to isEmpty(value) && typeof value !== 'boolean' ? null : [value];

When we have more custom fields in future, we can create some util function to handle different types.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you are right. Ok!


transformedCustomFields.push({
key: configCustomField.key,
type: configCustomField.type,
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/cases/public/components/create/translations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ export const STEP_THREE_TITLE = i18n.translate('xpack.cases.create.stepThreeTitl
defaultMessage: 'External Connector Fields',
});

export const ADDITIONAL_FIELDS = i18n.translate('xpack.cases.create.additionalFields', {
defaultMessage: 'Additional fields',
});

export const SYNC_ALERTS_LABEL = i18n.translate('xpack.cases.create.syncAlertsLabel', {
defaultMessage: 'Sync alert status with case status',
});
Expand Down
Loading