From fc947242e01a7a9dae7afd3eb97b78690078e3f5 Mon Sep 17 00:00:00 2001 From: Janki Salvi Date: Fri, 21 Jun 2024 11:11:49 +0100 Subject: [PATCH 1/4] initial commit --- .../server/client/configure/client.test.ts | 126 ++++++++ .../cases/server/client/configure/client.ts | 9 +- .../plugins/cases/server/client/utils.test.ts | 286 ++++++++++++++++++ x-pack/plugins/cases/server/client/utils.ts | 35 +++ 4 files changed, 454 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/cases/server/client/configure/client.test.ts b/x-pack/plugins/cases/server/client/configure/client.test.ts index dc07909cb3256..8b312d2d957a2 100644 --- a/x-pack/plugins/cases/server/client/configure/client.test.ts +++ b/x-pack/plugins/cases/server/client/configure/client.test.ts @@ -788,6 +788,132 @@ describe('client', () => { 'Failed to get patch configure in route: Error: The following custom fields have the wrong type in the request: "text label"' ); }); + + it('removes deleted custom field from template correctly', async () => { + clientArgs.services.caseConfigureService.get.mockResolvedValue({ + // @ts-ignore: these are all the attributes needed for the test + attributes: { + connector: { + id: 'none', + name: 'none', + type: ConnectorTypes.none, + fields: null, + }, + customFields: [ + { + key: 'custom_field_key_1', + label: 'text label', + type: CustomFieldTypes.TEXT, + required: false, + }, + ], + templates: [ + { + key: 'template_1', + name: 'template 1', + description: 'this is test description', + caseFields: { + customFields: [ + { + key: 'custom_field_key_1', + type: CustomFieldTypes.TEXT, + value: 'custom field value 1', + }, + ], + }, + }, + ], + closure_type: 'close-by-user', + owner: 'cases', + }, + id: 'test-id', + version: 'test-version', + }); + + await update( + 'test-id', + { + version: 'test-version', + customFields: [], + templates: [ + { + key: 'template_1', + name: 'template 1', + description: 'this is test description', + caseFields: { + customFields: [ + { + key: 'custom_field_key_1', + type: CustomFieldTypes.TEXT, + value: 'updated value', + }, + ], + }, + }, + ], + }, + clientArgs, + casesClientInternal + ); + + expect(clientArgs.services.caseConfigureService.patch).toHaveBeenCalledWith({ + configurationId: 'test-id', + originalConfiguration: { + attributes: { + closure_type: 'close-by-user', + connector: { + fields: null, + id: 'none', + name: 'none', + type: '.none', + }, + customFields: [ + { + key: 'custom_field_key_1', + label: 'text label', + required: false, + type: 'text', + }, + ], + owner: 'cases', + templates: [ + { + caseFields: { + customFields: [ + { + key: 'custom_field_key_1', + type: 'text', + value: 'custom field value 1', + }, + ], + }, + description: 'this is test description', + key: 'template_1', + name: 'template 1', + }, + ], + }, + id: 'test-id', + version: 'test-version', + }, + unsecuredSavedObjectsClient: expect.anything(), + updatedAttributes: { + customFields: [], + templates: [ + { + caseFields: { + customFields: [], + }, + description: 'this is test description', + key: 'template_1', + name: 'template 1', + }, + ], + updated_at: expect.anything(), + updated_by: expect.anything(), + }, + }); + }); }); describe('assignees', () => { diff --git a/x-pack/plugins/cases/server/client/configure/client.ts b/x-pack/plugins/cases/server/client/configure/client.ts index 9d202b87b0a2c..68db617af8bc2 100644 --- a/x-pack/plugins/cases/server/client/configure/client.ts +++ b/x-pack/plugins/cases/server/client/configure/client.ts @@ -44,7 +44,7 @@ import type { CasesClientArgs } from '../types'; import { getMappings } from './get_mappings'; import { Operations } from '../../authorization'; -import { combineAuthorizedAndOwnerFilter } from '../utils'; +import { combineAuthorizedAndOwnerFilter, removeCustomFieldFromTemplates } from '../utils'; import type { MappingsArgs, CreateMappingsArgs, UpdateMappingsArgs } from './types'; import { createMappings } from './create_mappings'; import { updateMappings } from './update_mappings'; @@ -326,6 +326,11 @@ export async function update( customFields: configuration.attributes.customFields, }); + const updatedTemplates = removeCustomFieldFromTemplates({ + templates, + customFields: request.customFields, + }); + await authorization.ensureAuthorized({ operation: Operations.updateConfiguration, entities: [{ owner: configuration.attributes.owner, id: configuration.id }], @@ -381,7 +386,7 @@ export async function update( configurationId: configuration.id, updatedAttributes: { ...queryWithoutVersionAndConnector, - ...(templates && { templates }), + ...(updatedTemplates && { templates: updatedTemplates }), ...(connector != null && { connector }), updated_at: updateDate, updated_by: user, diff --git a/x-pack/plugins/cases/server/client/utils.test.ts b/x-pack/plugins/cases/server/client/utils.test.ts index 8f9e8648a1269..b1d3e8e1606ba 100644 --- a/x-pack/plugins/cases/server/client/utils.test.ts +++ b/x-pack/plugins/cases/server/client/utils.test.ts @@ -19,6 +19,7 @@ import { constructQueryOptions, constructSearch, convertSortField, + removeCustomFieldFromTemplates, } from './utils'; import { CasePersistedSeverity, CasePersistedStatus } from '../common/types/case'; import type { CustomFieldsConfiguration } from '../../common/types/domain'; @@ -1130,4 +1131,289 @@ describe('utils', () => { ); }); }); + + describe('removeCustomFieldFromTemplates', () => { + const customFields = [ + { + type: CustomFieldTypes.TEXT as const, + key: 'test_key_1', + label: 'My test label 1', + required: true, + defaultValue: 'My default value', + }, + { + type: CustomFieldTypes.TOGGLE as const, + key: 'test_key_2', + label: 'My test label 2', + required: true, + defaultValue: true, + }, + { + type: CustomFieldTypes.TEXT as const, + key: 'test_key_3', + label: 'My test label 3', + required: false, + }, + ]; + + const templates = [ + { + key: 'test_template_1', + name: 'First test template', + description: 'This is a first test template', + caseFields: { + customFields: [ + { + type: CustomFieldTypes.TEXT as const, + key: 'test_key_1', + value: 'My default value', + }, + { + type: CustomFieldTypes.TOGGLE as const, + key: 'test_key_2', + value: false, + }, + { + type: CustomFieldTypes.TEXT as const, + key: 'test_key_3', + value: 'Test custom field', + }, + ], + }, + }, + { + key: 'test_template_2', + name: 'Second test template', + description: 'This is a second test template', + tags: [], + caseFields: { + customFields: [ + { + type: CustomFieldTypes.TEXT as const, + key: 'test_key_1', + value: 'My value', + }, + { + type: CustomFieldTypes.TOGGLE as const, + key: 'test_key_2', + value: true, + }, + ], + }, + }, + ]; + + it('removes custom field from template correctly', () => { + const res = removeCustomFieldFromTemplates({ + templates, + customFields: [customFields[0], customFields[1]], + }); + + expect(res).toEqual([ + { + caseFields: { + customFields: [ + { + key: 'test_key_1', + type: 'text', + value: 'My default value', + }, + { + key: 'test_key_2', + type: 'toggle', + value: false, + }, + ], + }, + description: 'This is a first test template', + key: 'test_template_1', + name: 'First test template', + }, + { + description: 'This is a second test template', + key: 'test_template_2', + name: 'Second test template', + tags: [], + caseFields: { + customFields: [ + { + key: 'test_key_1', + type: 'text', + value: 'My value', + }, + { + key: 'test_key_2', + type: 'toggle', + value: true, + }, + ], + }, + }, + ]); + }); + + it('removes multiple custom fields from template correctly', () => { + const res = removeCustomFieldFromTemplates({ + templates, + customFields: [customFields[0]], + }); + + expect(res).toEqual([ + { + caseFields: { + customFields: [ + { + key: 'test_key_1', + type: 'text', + value: 'My default value', + }, + ], + }, + description: 'This is a first test template', + key: 'test_template_1', + name: 'First test template', + }, + { + description: 'This is a second test template', + key: 'test_template_2', + name: 'Second test template', + tags: [], + caseFields: { + customFields: [ + { + key: 'test_key_1', + type: 'text', + value: 'My value', + }, + ], + }, + }, + ]); + }); + + it('removes all custom fields from templates when custom fields are empty', () => { + const res = removeCustomFieldFromTemplates({ + templates, + customFields: [], + }); + + expect(res).toEqual([ + { + caseFields: { + customFields: [], + }, + description: 'This is a first test template', + key: 'test_template_1', + name: 'First test template', + }, + { + description: 'This is a second test template', + key: 'test_template_2', + name: 'Second test template', + tags: [], + caseFields: { + customFields: [], + }, + }, + ]); + }); + + it('removes all custom fields from templates when custom fields are undefined', () => { + const res = removeCustomFieldFromTemplates({ + templates, + customFields: undefined, + }); + + expect(res).toEqual([ + { ...templates[0], caseFields: { customFields: [] } }, + { ...templates[1], caseFields: { ...templates[1].caseFields, customFields: [] } }, + ]); + }); + + it('does not remove custom field when templates do not have custom fields', () => { + const res = removeCustomFieldFromTemplates({ + templates: [ + { + key: 'test_template_1', + name: 'First test template', + description: 'This is a first test template', + caseFields: null, + }, + { + key: 'test_template_2', + name: 'Second test template', + caseFields: { + title: 'Test title', + description: 'this is test', + }, + }, + ], + customFields: [customFields[0], customFields[1]], + }); + + expect(res).toEqual([ + { + caseFields: null, + description: 'This is a first test template', + key: 'test_template_1', + name: 'First test template', + }, + { + key: 'test_template_2', + name: 'Second test template', + caseFields: { + description: 'this is test', + title: 'Test title', + }, + }, + ]); + }); + + it('does not remove custom field when templates have empty custom fields', () => { + const res = removeCustomFieldFromTemplates({ + templates: [ + { + key: 'test_template_2', + name: 'Second test template', + caseFields: { + title: 'Test title', + description: 'this is test', + customFields: [], + }, + }, + ], + customFields: [customFields[0], customFields[1]], + }); + + expect(res).toEqual([ + { + key: 'test_template_2', + name: 'Second test template', + caseFields: { + title: 'Test title', + description: 'this is test', + customFields: [], + }, + }, + ]); + }); + + it('does not remove custom field from templates are empty', () => { + const res = removeCustomFieldFromTemplates({ + templates: [], + customFields: [customFields[0], customFields[1]], + }); + + expect(res).toEqual([]); + }); + + it('returns empty array when templates are undefined', () => { + const res = removeCustomFieldFromTemplates({ + templates: [], + customFields: [customFields[0], customFields[1]], + }); + + expect(res).toEqual([]); + }); + }); }); diff --git a/x-pack/plugins/cases/server/client/utils.ts b/x-pack/plugins/cases/server/client/utils.ts index 0ce4da8bcc21b..258761a563fd3 100644 --- a/x-pack/plugins/cases/server/client/utils.ts +++ b/x-pack/plugins/cases/server/client/utils.ts @@ -21,6 +21,7 @@ import type { CaseStatuses, CustomFieldsConfiguration, ExternalReferenceAttachmentPayload, + TemplatesConfiguration, } from '../../common/types/domain'; import { ActionsAttachmentPayloadRt, @@ -604,3 +605,37 @@ export const constructSearch = ( return { search }; }; + +/** + * remove deleted custom field from template + */ +export const removeCustomFieldFromTemplates = ({ + templates, + customFields, +}: { + templates?: TemplatesConfiguration; + customFields?: CustomFieldsConfiguration; +}): TemplatesConfiguration => { + if (!templates || !templates.length) { + return []; + } + + return templates.map((template) => { + if (!template.caseFields?.customFields || !template.caseFields?.customFields.length) { + return template; + } + + if (!customFields || !customFields?.length) { + return { ...template, caseFields: { ...template.caseFields, customFields: [] } }; + } + + const templateCustomFields = template.caseFields.customFields.filter((templateCustomField) => + customFields?.find((customField) => customField.key === templateCustomField.key) + ); + + return { + ...template, + caseFields: { ...template.caseFields, customFields: templateCustomFields }, + }; + }); +}; From 2aa619043aae29e77f098105ce92a9ab2ecb7273 Mon Sep 17 00:00:00 2001 From: Janki Salvi Date: Fri, 21 Jun 2024 14:04:23 +0100 Subject: [PATCH 2/4] added integration test --- .../tests/common/configure/patch_configure.ts | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/configure/patch_configure.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/configure/patch_configure.ts index 0b93aeb8c5ddd..2af8aa130b225 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/configure/patch_configure.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/configure/patch_configure.ts @@ -204,6 +204,7 @@ export default ({ getService }: FtrProviderContext): void => { }); const newConfiguration = await updateConfiguration(supertest, configuration.id, { version: configuration.version, + customFields: customFieldsConfiguration, templates, }); @@ -215,6 +216,78 @@ export default ({ getService }: FtrProviderContext): void => { }); }); + it('should remove custom feilds from templates', async () => { + const customFieldsConfiguration = [ + { + key: 'text_field_1', + type: CustomFieldTypes.TEXT, + label: 'Text field 1', + required: true, + }, + { + key: 'toggle_field_1', + label: '#2', + type: CustomFieldTypes.TOGGLE, + required: false, + }, + ]; + + const templates = [ + { + key: 'test_template_2', + name: 'Second test template', + description: 'This is a second test template', + caseFields: { + title: 'Case with sample template 2', + description: 'case desc', + severity: CaseSeverity.LOW, + category: null, + tags: ['sample-4'], + assignees: [], + customFields: [ + { + key: 'text_field_1', + type: CustomFieldTypes.TEXT, + value: 'this is a text field value', + }, + { + key: 'toggle_field_1', + value: true, + type: CustomFieldTypes.TOGGLE, + }, + ], + connector: { + id: 'none', + name: 'My Connector', + type: ConnectorTypes.none, + fields: null, + }, + }, + }, + ]; + + const configuration = await createConfiguration(supertest, { + ...getConfigurationRequest(), + customFields: customFieldsConfiguration as ConfigurationPatchRequest['customFields'], + }); + + // delete custom fields + const newConfiguration = await updateConfiguration(supertest, configuration.id, { + version: configuration.version, + customFields: [], + templates: templates as ConfigurationPatchRequest['templates'], + }); + + const data = removeServerGeneratedPropertiesFromSavedObject(newConfiguration); + expect(data).to.eql({ + ...getConfigurationOutput(true), + customFields: [], + templates: [ + { ...templates[0], caseFields: { ...templates[0].caseFields, customFields: [] } }, + ], + }); + }); + describe('validation', () => { it('should not patch a configuration with unsupported connector type', async () => { const configuration = await createConfiguration(supertest); From 59203e20abacce85f0f7253aeb694d20ddd49c6c Mon Sep 17 00:00:00 2001 From: Janki Salvi Date: Mon, 24 Jun 2024 11:06:36 +0100 Subject: [PATCH 3/4] fix typo --- .../tests/common/configure/patch_configure.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/configure/patch_configure.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/configure/patch_configure.ts index 2af8aa130b225..114182a1ad20d 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/configure/patch_configure.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/configure/patch_configure.ts @@ -216,7 +216,7 @@ export default ({ getService }: FtrProviderContext): void => { }); }); - it('should remove custom feilds from templates', async () => { + it('should remove custom fields from templates', async () => { const customFieldsConfiguration = [ { key: 'text_field_1', From 73c34eebf6373ce64df35cecbd4c9b7362b91070 Mon Sep 17 00:00:00 2001 From: Janki Salvi Date: Tue, 25 Jun 2024 10:34:36 +0100 Subject: [PATCH 4/4] PR feedback --- x-pack/plugins/cases/server/client/utils.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/cases/server/client/utils.test.ts b/x-pack/plugins/cases/server/client/utils.test.ts index b1d3e8e1606ba..56615189d1d5e 100644 --- a/x-pack/plugins/cases/server/client/utils.test.ts +++ b/x-pack/plugins/cases/server/client/utils.test.ts @@ -1398,7 +1398,7 @@ describe('utils', () => { ]); }); - it('does not remove custom field from templates are empty', () => { + it('does not remove custom field from empty templates', () => { const res = removeCustomFieldFromTemplates({ templates: [], customFields: [customFields[0], customFields[1]], @@ -1409,7 +1409,7 @@ describe('utils', () => { it('returns empty array when templates are undefined', () => { const res = removeCustomFieldFromTemplates({ - templates: [], + templates: undefined, customFields: [customFields[0], customFields[1]], });