Skip to content

Commit

Permalink
[Cases] Custom field default values in configuration forms (#174628)
Browse files Browse the repository at this point in the history
See #171747 for more info

## Summary

**Merging into a feature branch.**

This PR handles **only** the cases configuration page.

With these changes, users can now configure default values for custom
fields in the UI.

<details><summary>Text custom field</summary>
<img width="1728" alt="Screenshot 2024-01-12 at 13 56 58"
src="https://github.com/elastic/kibana/assets/1533137/972ee7b4-e5a3-4368-a499-914bde7f0bc1">
</details>

<details><summary>Toggle custom field</summary>
<img width="1728" alt="Screenshot 2024-01-12 at 14 01 43"
src="https://github.com/elastic/kibana/assets/1533137/5b8a1e0c-7d90-4f51-a107-ad989af645f3">

</details>

When clicking `required` the default value for a `toggle` custom field
is `false`.

When clicking `required` the default value for a `text` custom field is
an empty `string`.

<details><summary>Trying to save an empty `string` as the default value
of a `text` custom field should display an error message on the
form.</summary>
<img width="1728" alt="Screenshot 2024-01-12 at 14 00 39"
src="https://github.com/elastic/kibana/assets/1533137/e5925d15-801e-4da2-87ef-d4d6686290f4"></details>

Viewing previously created custom fields should display their default
values correctly.

Viewing previously created custom fields **in a version where default
values did not exist** should display the forms correctly.

## Other Notes

1. I changed the property name from `default_value` to `defaultValue`
2. I fixed some instances of `expect(screen.getById...` where I found
them.
3. I addressed most comments left [from the previous
PR](#174043). The last one will be
a separate PR(moving some types around).
  • Loading branch information
adcoelho authored Jan 18, 2024
1 parent 754d8d7 commit 8dc8647
Show file tree
Hide file tree
Showing 46 changed files with 641 additions and 336 deletions.
23 changes: 6 additions & 17 deletions x-pack/plugins/cases/common/types/api/configure/v1.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ describe('configure', () => {
label: 'Text Custom Field',
type: CustomFieldTypes.TEXT,
required: true,
default_value: 'foobar',
defaultValue: 'foobar',
};

it('has expected attributes in request', () => {
Expand All @@ -327,18 +327,7 @@ describe('configure', () => {
PathReporter.report(
TextCustomFieldConfigurationRt.decode({
...defaultRequest,
default_value: false,
})
)[0]
).toContain('Invalid value false supplied');
});

it('limits the default_value to ', () => {
expect(
PathReporter.report(
TextCustomFieldConfigurationRt.decode({
...defaultRequest,
default_value: false,
defaultValue: false,
})
)[0]
).toContain('Invalid value false supplied');
Expand All @@ -349,7 +338,7 @@ describe('configure', () => {
PathReporter.report(
TextCustomFieldConfigurationRt.decode({
...defaultRequest,
default_value: '#'.repeat(MAX_CUSTOM_FIELD_TEXT_VALUE_LENGTH + 1),
defaultValue: '#'.repeat(MAX_CUSTOM_FIELD_TEXT_VALUE_LENGTH + 1),
})
)[0]
).toContain(
Expand All @@ -362,7 +351,7 @@ describe('configure', () => {
PathReporter.report(
TextCustomFieldConfigurationRt.decode({
...defaultRequest,
default_value: '',
defaultValue: '',
})
)[0]
).toContain('The value field cannot be an empty string.');
Expand All @@ -375,7 +364,7 @@ describe('configure', () => {
label: 'Toggle Custom Field',
type: CustomFieldTypes.TOGGLE,
required: true,
default_value: false,
defaultValue: false,
};

it('has expected attributes in request', () => {
Expand All @@ -401,7 +390,7 @@ describe('configure', () => {
PathReporter.report(
ToggleCustomFieldConfigurationRt.decode({
...defaultRequest,
default_value: 'foobar',
defaultValue: 'foobar',
})
)[0]
).toContain('Invalid value "foobar" supplied');
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/cases/common/types/api/configure/v1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const TextCustomFieldConfigurationRt = rt.intersection([
CustomFieldConfigurationWithoutTypeRt,
rt.exact(
rt.partial({
default_value: rt.union([CaseCustomFieldTextWithValidationValueRt, rt.null]),
defaultValue: rt.union([CaseCustomFieldTextWithValidationValueRt, rt.null]),
})
),
]);
Expand All @@ -52,7 +52,7 @@ export const ToggleCustomFieldConfigurationRt = rt.intersection([
CustomFieldConfigurationWithoutTypeRt,
rt.exact(
rt.partial({
default_value: rt.union([rt.boolean, rt.null]),
defaultValue: rt.union([rt.boolean, rt.null]),
})
),
]);
Expand Down
65 changes: 62 additions & 3 deletions x-pack/plugins/cases/common/types/domain/configure/v1.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import { PathReporter } from 'io-ts/lib/PathReporter';
import { ConnectorTypes } from '../connector/v1';
import { CustomFieldTypes } from '../custom_field/v1';
import {
Expand Down Expand Up @@ -192,10 +193,10 @@ describe('configure', () => {
key: 'my_text_custom_field',
label: 'Text Custom Field',
type: CustomFieldTypes.TEXT,
required: true,
required: false,
};

it('has expected attributes in request', () => {
it('has expected attributes in request with required: false', () => {
const query = TextCustomFieldConfigurationRt.decode(defaultRequest);

expect(query).toStrictEqual({
Expand All @@ -204,6 +205,35 @@ describe('configure', () => {
});
});

it('has expected attributes in request with defaultValue and required: true', () => {
const query = TextCustomFieldConfigurationRt.decode({
...defaultRequest,
required: true,
defaultValue: 'foobar',
});

expect(query).toStrictEqual({
_tag: 'Right',
right: {
...defaultRequest,
required: true,
defaultValue: 'foobar',
},
});
});

it('defaultValue fails if the type is not string', () => {
expect(
PathReporter.report(
TextCustomFieldConfigurationRt.decode({
...defaultRequest,
required: true,
defaultValue: false,
})
)[0]
).toContain('Invalid value false supplied');
});

it('removes foo:bar attributes from request', () => {
const query = TextCustomFieldConfigurationRt.decode({ ...defaultRequest, foo: 'bar' });

Expand All @@ -222,7 +252,7 @@ describe('configure', () => {
required: false,
};

it('has expected attributes in request', () => {
it('has expected attributes in request with required: false', () => {
const query = ToggleCustomFieldConfigurationRt.decode(defaultRequest);

expect(query).toStrictEqual({
Expand All @@ -231,6 +261,35 @@ describe('configure', () => {
});
});

it('has expected attributes in request with defaultValue and required: true', () => {
const query = ToggleCustomFieldConfigurationRt.decode({
...defaultRequest,
required: true,
defaultValue: false,
});

expect(query).toStrictEqual({
_tag: 'Right',
right: {
...defaultRequest,
required: true,
defaultValue: false,
},
});
});

it('defaultValue fails if the type is not boolean', () => {
expect(
PathReporter.report(
ToggleCustomFieldConfigurationRt.decode({
...defaultRequest,
required: true,
defaultValue: 'foobar',
})
)[0]
).toContain('Invalid value "foobar" supplied');
});

it('removes foo:bar attributes from request', () => {
const query = ToggleCustomFieldConfigurationRt.decode({ ...defaultRequest, foo: 'bar' });

Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/cases/common/types/domain/configure/v1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const TextCustomFieldConfigurationRt = rt.intersection([
CustomFieldConfigurationWithoutTypeRt,
rt.exact(
rt.partial({
default_value: rt.union([rt.string, rt.null]),
defaultValue: rt.union([rt.string, rt.null]),
})
),
]);
Expand All @@ -45,7 +45,7 @@ export const ToggleCustomFieldConfigurationRt = rt.intersection([
CustomFieldConfigurationWithoutTypeRt,
rt.exact(
rt.partial({
default_value: rt.union([rt.boolean, rt.null]),
defaultValue: rt.union([rt.boolean, rt.null]),
})
),
]);
Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/cases/public/common/test_utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ export const FormTestComponent: React.FC<FormTestComponentProps> = ({
return (
<Form form={form}>
{children}
<EuiButton onClick={() => form.submit()}>{'Submit'}</EuiButton>
<EuiButton onClick={() => form.submit()} data-test-subj="form-test-component-submit-button">
{'Submit'}
</EuiButton>
</Form>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ describe('Case View Page files tab', () => {
/>
);

expect(screen.getByTestId('case-custom-field-wrapper-test_key_1')).toBeInTheDocument();
expect(screen.getByTestId('case-custom-field-wrapper-test_key_2')).toBeInTheDocument();
expect(await screen.findByTestId('case-custom-field-wrapper-test_key_1')).toBeInTheDocument();
expect(await screen.findByTestId('case-custom-field-wrapper-test_key_2')).toBeInTheDocument();
});

it('should render the custom fields types when the custom fields are empty', async () => {
Expand All @@ -52,11 +52,11 @@ describe('Case View Page files tab', () => {
/>
);

expect(screen.getByTestId('case-custom-field-wrapper-test_key_1')).toBeInTheDocument();
expect(screen.getByTestId('case-custom-field-wrapper-test_key_2')).toBeInTheDocument();
expect(await screen.findByTestId('case-custom-field-wrapper-test_key_1')).toBeInTheDocument();
expect(await screen.findByTestId('case-custom-field-wrapper-test_key_2')).toBeInTheDocument();
});

it('should not show the custom fields if the configuration is empty', async () => {
it('should not show the custom fields if the configuration is empty', () => {
appMockRender.render(
<CustomFields
isLoading={false}
Expand All @@ -82,12 +82,24 @@ describe('Case View Page files tab', () => {
/>
);

const customFields = screen.getAllByTestId('case-custom-field-wrapper', { exact: false });
const customFields = await screen.findAllByTestId('case-custom-field-wrapper', {
exact: false,
});

expect(customFields.length).toBe(2);
expect(customFields.length).toBe(4);

expect(within(customFields[0]).getByRole('heading')).toHaveTextContent('My test label 1');
expect(within(customFields[1]).getByRole('heading')).toHaveTextContent('My test label 2');
expect(await within(customFields[0]).findByRole('heading')).toHaveTextContent(
'My test label 1'
);
expect(await within(customFields[1]).findByRole('heading')).toHaveTextContent(
'My test label 2'
);
expect(await within(customFields[2]).findByRole('heading')).toHaveTextContent(
'My test label 3'
);
expect(await within(customFields[3]).findByRole('heading')).toHaveTextContent(
'My test label 4'
);
});

it('pass the permissions to custom fields correctly', async () => {
Expand Down Expand Up @@ -117,16 +129,14 @@ describe('Case View Page files tab', () => {
/>
);

userEvent.click(screen.getByRole('switch'));
userEvent.click((await screen.findAllByRole('switch'))[0]);

await waitFor(() => {
expect(onSubmit).toBeCalledWith([
{
type: CustomFieldTypes.TEXT,
key: 'test_key_1',
value: null,
},
{ type: CustomFieldTypes.TEXT, key: 'test_key_1', value: null },
{ type: CustomFieldTypes.TOGGLE, key: 'test_key_2', value: true },
customFieldsMock[2],
customFieldsMock[3],
]);
});
});
Expand All @@ -141,16 +151,14 @@ describe('Case View Page files tab', () => {
/>
);

userEvent.click(screen.getByRole('switch'));
userEvent.click((await screen.findAllByRole('switch'))[0]);

await waitFor(() => {
expect(onSubmit).toBeCalledWith([
{
type: CustomFieldTypes.TEXT,
key: 'test_key_1',
value: null,
},
{ type: CustomFieldTypes.TEXT, key: 'test_key_1', value: null },
{ type: CustomFieldTypes.TOGGLE, key: 'test_key_2', value: false },
customFieldsMock[2],
customFieldsMock[3],
]);
});
});
Expand All @@ -172,7 +180,7 @@ describe('Case View Page files tab', () => {
/>
);

userEvent.click(screen.getByRole('switch'));
userEvent.click(await screen.findByRole('switch'));

await waitFor(() => {
expect(onSubmit).toBeCalledWith([
Expand All @@ -191,12 +199,14 @@ describe('Case View Page files tab', () => {
/>
);

userEvent.click(screen.getByRole('switch'));
userEvent.click((await screen.findAllByRole('switch'))[0]);

await waitFor(() => {
expect(onSubmit).toBeCalledWith([
customFieldsMock[0],
{ type: CustomFieldTypes.TOGGLE, key: 'test_key_2', value: false },
customFieldsMock[2],
customFieldsMock[3],
]);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,11 @@ describe('ConfigureCases', () => {
fields: null,
},
closureType: 'close-by-user',
customFields: [{ ...customFieldsConfigurationMock[1] }],
customFields: [
{ ...customFieldsConfigurationMock[1] },
{ ...customFieldsConfigurationMock[2] },
{ ...customFieldsConfigurationMock[3] },
],
id: '',
version: '',
});
Expand All @@ -728,9 +732,7 @@ describe('ConfigureCases', () => {
expect(await screen.findByTestId('custom-field-flyout')).toBeInTheDocument();

userEvent.paste(screen.getByTestId('custom-field-label-input'), '!!');

userEvent.click(screen.getByTestId('text-custom-field-options'));

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

await waitFor(() => {
Expand All @@ -744,11 +746,14 @@ describe('ConfigureCases', () => {
closureType: 'close-by-user',
customFields: [
{
...customFieldsConfigurationMock[0],
key: customFieldsConfigurationMock[0].key,
type: customFieldsConfigurationMock[0].type,
label: `${customFieldsConfigurationMock[0].label}!!`,
required: !customFieldsConfigurationMock[0].required,
},
{ ...customFieldsConfigurationMock[1] },
{ ...customFieldsConfigurationMock[2] },
{ ...customFieldsConfigurationMock[3] },
],
id: '',
version: '',
Expand Down
Loading

0 comments on commit 8dc8647

Please sign in to comment.